then / is-promise

Test whether an object looks like a promises-a+ promise
MIT License
282 stars 32 forks source link

Breaking change in minor version #19

Closed ljharb closed 4 years ago

ljharb commented 4 years ago

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 commented 4 years ago

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

RyanZim commented 4 years ago

@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 commented 4 years ago

If you are on NPM, run: npm install is-promise@2.1.0 --save --save-exact

ForbesLindesay commented 4 years ago

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

jakejarvis commented 4 years ago

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 commented 4 years ago

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

This includes:

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

ForbesLindesay commented 4 years ago

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 commented 4 years ago

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