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

Breaking change in minor version #19

Closed
ljharb opened this issue Apr 25, 2020 · 8 comments
Closed

Breaking change in minor version #19

ljharb opened this issue Apr 25, 2020 · 8 comments

Comments

@ljharb
Copy link

ljharb commented Apr 25, 2020

Adding "exports" is a breaking change, and should not have happened in v2.2.0. Please release a new v2 that does not have "exports" and does not use type:module or ESM, and then rerelease as v3.

@FritzTheDev
Copy link

👍 this... The changes break a fresh husky/lint-staged/prettier setup for me and I can only imagine countless other things.

@RyanZim
Copy link
Contributor

RyanZim commented Apr 25, 2020

@ljharb How is adding exports breaking? I realize exports changes the behavior of packagename/somepath, but there are no other published files to import for is-promise. What am I missing?

@MarkKragerup
Copy link

If you are on NPM, run:
npm install [email protected] --save --save-exact

@ForbesLindesay
Copy link
Member

Yes. How is this braking. Adding exports should be backwards compatible

@jakejarvis
Copy link

jakejarvis commented Apr 25, 2020

example logs w/ lighthouse-ci failing, for what it's worth: https://github.com/jakejarvis/jarv.is/runs/618312068#step:6:9

internal/modules/cjs/loader.js:1172
      throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /opt/hostedtoolcache/node/12.16.2/x64/lib/node_modules/@lhci/cli/node_modules/is-promise/index.js
require() of ES modules is not supported.
require() of /opt/hostedtoolcache/node/12.16.2/x64/lib/node_modules/@lhci/cli/node_modules/is-promise/index.js from /opt/hostedtoolcache/node/12.16.2/x64/lib/node_modules/@lhci/cli/node_modules/run-async/index.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 /opt/hostedtoolcache/node/12.16.2/x64/lib/node_modules/@lhci/cli/node_modules/is-promise/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /opt/hostedtoolcache/node/12.16.2/x64/lib/node_modules/@lhci/cli/node_modules/is-promise/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1172:13)

edit: just saw a fix was pushed, thanks for the prompt response @RyanZim!

@ljharb
Copy link
Author

ljharb commented Apr 25, 2020

If in fact every single possible require path in v2.1 is available in v2.2, then it’s not breaking.

This includes:

  • is-promise
  • is-promise/index
  • is-promise/index.js
  • is-promise/package
  • is-promise/package.json

Adding exports is intended to be a breaking change. Adding ESM after that is what’s not supposed to have to be breaking.

@ForbesLindesay
Copy link
Member

It should now be resolved. Since there are 4 issues about this, I'm going to close this issue in favour of #20. Please comment there if 2.2.1 does not fix your issue.

@rcenamsi
Copy link

Thanks! at first glance in v2.2.0 there is no .mjs file..

@then then locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants