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

[Bug]: Enforcing import assertions should have been a BREAKING CHANGE, made worse by inability to pin jest to a specific version, (and made yet worse by TC39's decision to demote import-assertions to stage 2) #13909

Closed
broofa opened this issue Feb 14, 2023 · 10 comments · Fixed by #13911

Comments

@broofa
Copy link
Contributor

broofa commented Feb 14, 2023

Version

29.4.2 (i.e. latest)

Steps to reproduce

[My apologies for a long report here but as the title says, I've run into multiple related issues that I think are best presented as a whole.]

Setup: Create a directory with the following three files:

package.json

{
  "type": "module",
  "devDependencies": { "jest": "29" },
  "scripts": {
    "test": "NODE_OPTIONS=--experimental-vm-modules jest"
  }
}

data.json

{ "hello": "world" }

index.spec.js

import data from './data.json';
console.log('Imported data', data);

it('works with json imports', function() {})

Issue 1: Releasing a BREAKING CHANGE in a minor update

From the release notes, version 29.4.0 introduced this change:

[jest-runtime] Enforce import assertions when importing JSON in ESM (#12755 & #13805)

In the directory you've just created...

$ npm install

$ npm test

 FAIL  ./index.spec.js
  ● Test suite failed to run

    Module "file:///private/tmp/foo/data.json" needs an import assertion of type "json"

      at Runtime.validateImportAssertions (node_modules/jest-runtime/build/index.js:624:25)
          at async Promise.all (index 0)

Test Suites: 1 failed, 1 total

As expected from the release notes, the test fails due to the missing import assertion. But this used to work in previous minor versions! For example ...

$ npm i -D jest@29.3

$ jest --version
29.4.2

Wait, what?!? I specifically installed v29.3. Why do I still have 29.4.2??

This brings us to ...

Issue 2: Can't install previous minor versions of jest

Apparently this is a thiing with jest? No matter what minor version you install, you end up getting the latest minor version for it? npm i jest@27.0.0 installs 27.5.1? npm i jest@28.0.0 installs 28.1.3?

I won't bore you with the details, but for others who might be reading this, it has to do with how jest is just a thin wrapper around a whole ecosystem of "...jest..." modules that all depend on semver "^"-prefixed dependencies, and how that manifests when installing modules.

This is both unexpected and magical. And, in the case of issue 1, above, not a little bit painful. It mean there's no viable way of reverting to the most recent version of jest that didn't have this behavior (v29.3.1). The most recent "pinnable" version is v28.1.3, which breaks stuff for a bunch of other reasons.

Still with me? 😄

We can, however, sort of revert to 29.3.1 by manually overriding everything via NPM's overrides feature. To do so, add the following overrides section to the package.json file:

  "overrides": {
    "@jest/console": "29.3",
    "@jest/core": "29.3",
    "@jest/environment": "29.3",
    "@jest/expect": "29.3",
    "@jest/expect-utils": "29.3",
    "@jest/fake-timers": "29.3",
    "@jest/globals": "29.3",
    "@jest/reporters": "29.3",
    "@jest/schemas": "29",
    "@jest/source-map": "29",
    "@jest/test-result": "29.3",
    "@jest/test-sequencer": "29.3",
    "@jest/transform": "29.3",
    "@jest/types": "29.3",
    "babel-jest": "29.3",
    "babel-plugin-jest-hoist": "29",
    "babel-preset-jest": "29",
    "diff-sequences": "29.3",
    "expect": "29.3",
    "jest-changed-files": "29",
    "jest-circus": "29.3",
    "jest-config": "29.3",
    "jest-diff": "29.3",
    "jest-docblock": "29",
    "jest-each": "29.3",
    "jest-environment-node": "29.3",
    "jest-get-type": "29",
    "jest-haste-map": "29.3",
    "jest-leak-detector": "29.3",
    "jest-matcher-utils": "29.3",
    "jest-message-util": "29.3",
    "jest-mock": "29.3",
    "jest-regex-util": "29",
    "jest-resolve": "29.3",
    "jest-resolve-dependencies": "29.3",
    "jest-runner": "29.3",
    "jest-runtime": "29.3",
    "jest-snapshot": "29.3",
    "jest-util": "29.3",
    "jest-validate": "29.3",
    "jest-watcher": "29.3",
    "jest-worker": "29.3",
    "pretty-format": "29.3"
  }

This is the very definition of "fugly" - so much so I don't see being able to justify doing this as part of a real project - but it gets the job done for testing purposes here ...

$ \rm package-lock.json && npm install

$ jest --version
29.3.1  #  YAY! It worked!

$ npm test
 PASS  ./index.spec.js
  ✓ works with json imports (1 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.454 s
Ran all test suites.

**NOTICE: Our test passes under 29.3.1... meaning (and I hope you'll agree) that this does in fact represent a BREAKING CHANGE which should have been released as v30.

Issue 3: Import assertions have been demoted from TC39 stage 3 back to stage 2

To complicate matters further, while poking around with this issue, I noticed this statement that TC39 added to the import-assertions README a couple days ago:

⚠️ This proposal has been demoted from Stage 3 to Stage 2 in January 2023 due to whatwg/html#7233. TC39 is working on a solution, which will require relaxing the "assert only" semantics and potentially changing the syntax. Import assertions have already been shipped in some implementations, consider the current instability when using them.

Expected behavior

  • Don't release breaking changes as minor updates
  • Provide a way to easily pin jest to specific minor versions
  • Revert the requirement for import assertions in light of the proposal having been demoted

I'll leave it at this, as I expect you folks will have already discussed some of this, or need to discuss this further.

Actual behavior

See previous sections...

Additional context

No response

Environment

System:
    OS: macOS 13.0
    CPU: (10) x64 Apple M1 Max
  Binaries:
    Node: 16.18.0 - ~/.nvm/versions/node/v16.18.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.18.0/bin/yarn
    npm: 8.19.3 - ~/.nvm/versions/node/v16.18.0/bin/npm
  npmPackages:
    jest: 29 => 29.4.2
@broofa
Copy link
Contributor Author

broofa commented Feb 14, 2023

In thinking further about this, I believe semver provides clear direction for how to proceed here:

As soon as you realize that you’ve broken the Semantic Versioning spec, fix the problem and release a new minor version that corrects the problem and restores backwards compatibility. Even under this circumstance, it is unacceptable to modify versioned releases. If it’s appropriate, document the offending version and inform your users of the problem so that they are aware of the offending version.

So... revert the import assertions changes (#12755 & #13805) and roll out as a patch release (ASAP)? And possibly npm deprecate previous 29.4.x versions with a note saying they inadvertently introduced a breaking change.

@SimenB
Copy link
Member

SimenB commented Feb 14, 2023

Yeah, I noticed it being "demoted". Unlucky timing for us 😅 Reverting seems sensible. Would you be up for sending a PR?

Issue 2 is a long-standing (and, imo, separate) one. We have an issue somewhere - I'm down to lock down our monorepo deps so versioning is easy to lock down.

@broofa
Copy link
Contributor Author

broofa commented Feb 14, 2023

Yeah, I noticed it being "demoted". Unlucky timing for us 😅 Reverting seems sensible. Would you be up for sending a PR?

Put up #13911 (but need help resolving lint errors).

Issue 2 is a long-standing (and, imo, separate) one. We have an issue somewhere - I'm down to lock down our monorepo deps so versioning is easy to lock down.

Will this involve more than turning off semver save-prefixes for all dependencies (across all jest packages)?

@SimenB
Copy link
Member

SimenB commented Feb 14, 2023

Yeah, code change for exact versions is pretty simple - hasn't been done due to some folks preferring current approach.

@broofa
Copy link
Contributor Author

broofa commented Feb 14, 2023

some folks preferring current approach

Let me guess... something to do with minimizing jest's install footprint when it's depended upon by thousands of modules, each of which specifies a slightly different version than the last?

I guess I can respect that but, damn, it's a pain in the ass when something like this crops up.

@SimenB SimenB linked a pull request Feb 15, 2023 that will close this issue
@SimenB
Copy link
Member

SimenB commented Feb 15, 2023

https://github.com/facebook/jest/releases/tag/v29.4.3

@SimenB
Copy link
Member

SimenB commented Feb 15, 2023

some folks preferring current approach

Let me guess... something to do with minimizing jest's install footprint when it's depended upon by thousands of modules, each of which specifies a slightly different version than the last?

correct 😀

@SimenB
Copy link
Member

SimenB commented Feb 15, 2023

Note that at least npm has a before so you should be able to force older versions: https://docs.npmjs.com/cli/v9/using-npm/config#before

@broofa
Copy link
Contributor Author

broofa commented Feb 15, 2023

Note that at least npm has a before so you should be able to force older versions: https://docs.npmjs.com/cli/v9/using-npm/config#before

TIL.

I'm going to have to ponder how that affects dependency management workflows. 'Feels like the sort of thing that works in theory ("it lets you establish your dependencies in a lockfile"), but maybe not so well in practice ("lockfiles get rebuilt all the time").

Regardless, this is good to know, thanks!

@broofa broofa changed the title [Bug]: Enforcing import assertions should have been a BREAKING CHANGE, made worse by inability to *actually* pin jest to a specific version, (and made yet worse by TC39's decision to revert import assertions to stage 2) [Bug]: Enforcing import assertions should have been a BREAKING CHANGE, made worse by inability to pin jest to a specific version, (and made yet worse by TC39's decision to demote import-assertions to stage 2) Feb 15, 2023
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants