Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Module Configuration in package.json #53

Closed
TheBarn opened this issue Aug 21, 2023 · 2 comments
Closed

Incorrect Module Configuration in package.json #53

TheBarn opened this issue Aug 21, 2023 · 2 comments
Labels

Comments

@TheBarn
Copy link

TheBarn commented Aug 21, 2023

Overview:

The @mapbox/tiny-sdf package currently has a misconfiguration in its package.json file, which is causing compatibility issues for users in environments that expect CommonJS modules (require() syntax). This issue arises due to the use of the "type": "module" field in combination with the main field set to an ESM file and not a commonJS file. This is causing confusion and errors for users who are trying to use the package in environments that rely on CommonJS modules.

Error and context:

While upgrading my stack to next.js 12.3.4, deck.gl 8.9.25 and mapbox.gl 2.15.0 I encountered the following error :

Error: Must use import to load ES Module: node_modules/@mapbox/tiny-sdf/index.js
require() of ES modules is not supported.
require() of node_modules/@mapbox/tiny-sdf/index.js from node_modules/@deck.gl/layers/dist/es5/text-layer/font-atlas-manager.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from node_modules/@mapbox/tiny-sdf/package.json.

I do not know why webpack chose @deck.gl/layers/dist/es5 instead of @deck.gl/layers/dist/esm, but @deck.gl was expecting a commonJS file due to the "type: module" in @mapbox/tiny-sdf/package.json

Proposed solution:

To address this issue and enhance compatibility with a wider range of environments, I suggest the following changes to the package.json file of the @mapbox/tiny-sdf package:

Remove the "type": "module" Field: Since the index.js file is already an ESM file, specifying "type": "module" is redundant and causes compatibility issues in CommonJS environments.

Update the main and module Fields:

  • Set "main" to "index.js": This allows CommonJS environments to use the package without issues.
  • Set "module" to "index.mjs": This indicates the ESM entry point for environments that support ESM.

Duplicate index.js file to index.mjs file, then update index.js file to follow commonJS syntax.

Impact and Benefits:

By implementing this change, the @mapbox/tiny-sdf package will become more compatible with a broader range of environments. Users who rely on both CommonJS and ESM modules will be able to use the package seamlessly, without encountering module system conflicts or errors.

Steps to Reproduce:

Attempt to use the @mapbox/tiny-sdf package in a CommonJS-based environment (such as Node.js) by importing it using require().
Observe the error caused by the incompatibility between the package's "type": "module" field and the use of require().

@donmccurdy
Copy link

donmccurdy commented Mar 22, 2024

@deck.gl was expecting a commonJS file due to the "type: module" in @mapbox/tiny-sdf/package.json
...
Remove the "type": "module" Field: Since the index.js file is already an ESM file, specifying "type": "module" is redundant ...

A possible misunderstanding here. "type": "module" declares that any .js files in the package are ESM. If not specified, all .js files are assumed to be CommonJS, which as you mention, index.js is not. So I don't think anything is misconfigured, @mapbox/tiny-sdf is configured to correctly indicate it supports ESM but not CommonJS.

If there's interest in supporting both CommonJS and ESM (this would presumably require a build step) I'd be happy to contribute a PR with the maintainers' build tool of choice. I do understand the desire to simplify things and support only ESM, if not.

I do not know why webpack chose @deck.gl/layers/dist/es5 instead of @deck.gl/layers/dist/esm ...

In v8.x, deck.gl had only main and module defined. As of v9.x, exports is also defined, which is a more standardized approach to support both. Perhaps that may improve Webpack's choice?

@mourner
Copy link
Member

mourner commented Jun 26, 2024

So I don't think anything is misconfigured, @mapbox/tiny-sdf is configured to correctly indicate it supports ESM but not CommonJS.

Correct. I don't intend to support CommonJS in my projects anymore, so I'll close the issue. Projects that still operate in a CommonJS environment should use older tiny-sdf versions.

@mourner mourner closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants