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

[v3.2.0] commonjs compatibility with node's module resolution #1439

Closed
JMounier opened this issue Nov 23, 2021 · 11 comments
Closed

[v3.2.0] commonjs compatibility with node's module resolution #1439

JMounier opened this issue Nov 23, 2021 · 11 comments

Comments

@JMounier
Copy link

JMounier commented Nov 23, 2021

Issue

Importing dexie using commonjs syntax triggers an ERR_REQUIRE_ESM error with the new 3.2.0 version. This was not the case with version 3.0.3.

Use case

I discovered the issue in one of our CD job. We are testing that the packages (just released) have their dependencies correctly listed and can start in a minimal environment (no typescript or any framework).

The easy way to do so is to just import our package on its own and try to run the import with node. The test does not start any Dexie instance.

Step to reproduce

Install the dexie dependency

yarn init -y; yarn add dexie@3.2.0

Create a simple index.js:

echo "require('dexie');" > index.js

Run the script:

node index.js

output (with node 16) (equivalent error with node 12 and 14):

node:internal/modules/cjs/loader:979
    throw new ERR_REQUIRE_ESM(filename, true);
    ^

Error [ERR_REQUIRE_ESM]: require() of ES Module ./node-playground/node_modules/dexie/dist/modern/dexie.mjs not supported.
Instead change the require of ./node-playground/node_modules/dexie/dist/modern/dexie.mjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (./node-playground/test.js:1:1) {
  code: 'ERR_REQUIRE_ESM'
}

Diagnosis

This error is due to a new property (exports) added in the package.json. This field was not present in prior versions and overrides the behaviour of the main property.

Currently, the main property holds the path to the build supporting commonjs (dist/dexie.js) whereas exports maps to the modern build (./dist/modern/dexie.mjs).

Conclusion

Dexie is not meant to run in node.js and this use case is probably out of scope.
But the new property using a relative path seems fishy and looks like a side effect from another change.

dfahlander added a commit that referenced this issue Nov 24, 2021
@dfahlander
Copy link
Collaborator

dfahlander commented Nov 24, 2021

Thanks for a nice repro. I could resolve the issue by following the guidelines for the exports field to support also require. But the exports field has turned out to be a bit complex and I will need to have a beta out for a while with this new change to make sure it works in all scenarios. A beta will be be released in a few days.

@JMounier
Copy link
Author

JMounier commented Nov 24, 2021

Thanks for the quick reply. We will freeze dexie to version 3.0.3 while waiting for a new release

@dfahlander
Copy link
Collaborator

dfahlander commented Jan 5, 2022

Please try dexie@3.2.1-beta.2 that includes the extended "exports" field in package.json to make dexie adapt to being possible to require.

@jayaddison
Copy link
Contributor

jayaddison commented Jan 5, 2022

This was causing an issue during an import statement here after an attempt to upgrade to v3.2.0.

Confirmed that the import issue is resolved after upgrading beyond that to v3.2.1-beta.2.

@JMounier
Copy link
Author

JMounier commented Jan 5, 2022

I can also confirm that v3.2.1-beta.2 fixes the issue for the repro I provided.

@dfahlander
Copy link
Collaborator

dfahlander commented Jan 7, 2022

Closing as resolved in 3.2.1-beta.2

@JMounier
Copy link
Author

JMounier commented Jan 14, 2022

Do you know when a stable version (3.2.1 or 4.0.0) will be released?

@dfahlander
Copy link
Collaborator

dfahlander commented Jan 14, 2022

3.2.1 is in beta to have its grace period for this change + and PR #1398, but I see no reason yet to not release it as stable now. Might release it within 1-2 weeks if I would guess.

@dfahlander
Copy link
Collaborator

dfahlander commented Feb 16, 2022

Released in Dexie 3.2.1

CaptainAchab pushed a commit to TankerHQ/sdk-js that referenced this issue Feb 16, 2022
@jayaddison
Copy link
Contributor

jayaddison commented Feb 16, 2022

Thanks @dfahlander! Is there a corresponding release of dexie-observable (and any other plugins?)

@dfahlander
Copy link
Collaborator

dfahlander commented Feb 20, 2022

Thanks @dfahlander! Is there a corresponding release of dexie-observable (and any other plugins?)

I hope it shouldn't be needed. But this needs to be tested and there's a risk well have to do something to prohibit dual package hazard somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants