contentful-export icon indicating copy to clipboard operation
contentful-export copied to clipboard

Switch config file require to dynamic import

Open Brad-Turner opened this issue 7 months ago • 4 comments

This PR aims to improve ESM compatibility by removing the require call to load an external config file.

Brad-Turner avatar May 24 '25 01:05 Brad-Turner

Hey @elylucas , any chance you can have a look over this PR please 🙏🏼

Brad-Turner avatar May 29 '25 12:05 Brad-Turner

Hey @Brad-Turner. Thank you so much for the PR. We're taking a look at this right now. To help us with the code review could you give us some more information about the context driving these changes?

  1. Could you elaborate on the specific ESM compatibility issue this PR resolves? Are there particular environments or bundlers where the current implementation fails?
  2. Is there a concrete use case or build system (e.g. Vite, Webpack, Node in ESM mode) where the existing require usage breaks execution or causes errors? If so, could you provide steps or a minimal reproduction?

Niko-Berry-Contentful avatar Jun 12 '25 20:06 Niko-Berry-Contentful

Hey @Niko-Berry-Contentful ,

Sorry for the delayed response. Busy time planning my wedding 😅

  1. Yep so this PR isn't going magically add full support for ESM, but it is a necessary task to help get this package there. In ESM, require syntax does not exist (this is a feature of commonjs). Dynamic imports are supported in both ESM and commonjs runtimes.
  2. This is only an error if you are running a node script (using ESM) and go to import and run this code programmatically. eg:
import contentfulExport from 'contentful-export';

await contentfulExport({ /* settings, etc */ }); 

At my work, we often script exports, transforms and uploads and this is kind of annoying when I have to switch between runtimes to export from/import to contentful.

Brad-Turner avatar Jun 25 '25 11:06 Brad-Turner

Hey @Brad-Turner,

thank you for this PR. You are right, in a pure ESM environment we won't have require available.

I have a few issue with your changes:

  1. a lot of formatting is touched, makes it hard to review and we may overlook a newly introduced bug/change there. Any chance you clean up the reformattings and only provide the actual changes to async/await + import()
  2. we will have to still support CJS environments, so have to be sure we provide this library in ESM + CJS
  3. We are working on ESM support for this and our other libs and it will come very soon. We should combine my changes there with yours!

Thank you so far, Benedikt

Note to myself: We need to mirror this change to contentful-import as well

axe312ger avatar Jul 02 '25 12:07 axe312ger