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?]: pnp-esm.test.ts fails on node 22 #6267

Closed
1 task
rluvaton opened this issue May 2, 2024 · 3 comments · Fixed by #6268
Closed
1 task

[Bug?]: pnp-esm.test.ts fails on node 22 #6267

rluvaton opened this issue May 2, 2024 · 3 comments · Fixed by #6268
Labels
bug Something isn't working

Comments

@rluvaton
Copy link

rluvaton commented May 2, 2024

I'm wondering if it's node issue or yarn

Self-service

  • I'd be willing to implement a fix

Describe the bug

Seem like the test packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts is failing on node 22

To reproduce

run the test in node 22

Environment

System:
    OS: Linux 6.2 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (2) x64 AMD EPYC 7763 64-Core Processor
  Binaries:
    Node: 22.0.0 - /tmp/xfs-f68ecfb2/node
    Yarn: 4.2.1-dev - /tmp/xfs-f68ecfb2/yarn
    npm: 10.5.1 - ~/nvm/current/bin/npm

Additional context


  ● Plug'n'Play - ESM › it should resolve JSON modules with an import assertion

    Temporary fixture folder: /tmp/xfs-cc3b54c5/test

    expect(received).resolves.toMatchObject()

    Received promise rejected instead of resolved
    Rejected to value: [Error: Command failed: /usr/local/share/nvm/versions/node/v22.0.0/bin/node /workspaces/berry/packages/yarnpkg-cli/bundles/yarn.js node ./index.js
    file:///tmp/xfs-cc3b54c5/test/index.js:2
              import foo from './foo.json' assert { type: 'json' };
                                           ^^^^^^·
    SyntaxError: Unexpected identifier 'assert'
        at compileSourceTextModule (node:internal/modules/esm/utils:337:16)
        at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:18)
        at callTranslator (node:internal/modules/esm/loader:416:14)
        at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:422:30)
        at async link (node:internal/modules/esm/module_job:88:21)·
    Node.js v22.0.0··
    ===== stdout:·
    ```
    ```·
    ===== stderr:·
    ```
    file:///tmp/xfs-cc3b54c5/test/index.js:2
              import foo from './foo.json' assert { type: 'json' };
                                           ^^^^^^·
    SyntaxError: Unexpected identifier 'assert'
        at compileSourceTextModule (node:internal/modules/esm/utils:337:16)
        at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:18)
        at callTranslator (node:internal/modules/esm/loader:416:14)
        at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:422:30)
        at async link (node:internal/modules/esm/module_job:88:21)·
    Node.js v22.0.0
    ```·
    ]

      203 |       async ({path, run, source}) => {
      204 |         await expect(run(`install`)).resolves.toMatchObject({code: 0});
    > 205 |
          | ^
      206 |         await xfs.writeFilePromise(
      207 |           ppath.join(path, `index.js`),
      208 |           `import './foo';`,

      at expect (../../.yarn/cache/expect-npm-29.2.1-77e66f565d-9bc10fa9bf.zip/node_modules/expect/build/index.js:105:15)
      at pkg-tests-specs/sources/pnp-esm.test.ts:205:11
      at Object.<anonymous> (pkg-tests-core/sources/utils/tests.ts:671:11)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 2 skipped, 40 passed, 43 total
Snapshots:   0 total
Time:        42.47 s, estimated 43 s
@rluvaton rluvaton added the bug Something isn't working label May 2, 2024
@rluvaton
Copy link
Author

rluvaton commented May 2, 2024

@arcanis @merceyz @paul-soporan @larixer you are the contact in citgm :)

@rluvaton
Copy link
Author

rluvaton commented May 2, 2024

@merceyz
Copy link
Member

merceyz commented May 2, 2024

Thanks for the heads up, though would have expected these to fail before v22 was released.

EDIT: They did nodejs/node#52505 (comment).

arcanis pushed a commit that referenced this issue May 6, 2024
**What's the problem this PR addresses?**

In Node.js v22 import assertions were replaced with import attributes so
we need to add support for those as well.

Fixes #6267

**How did you fix it?**

Added support for the `importAttributes` property.

**Checklist**
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).
- [x] I have set the packages that need to be released for my changes to
be effective.
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
merceyz added a commit that referenced this issue May 8, 2024
**What's the problem this PR addresses?**

In Node.js v22 import assertions were replaced with import attributes so
we need to add support for those as well.

Fixes #6267

**How did you fix it?**

Added support for the `importAttributes` property.

**Checklist**
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).
- [x] I have set the packages that need to be released for my changes to
be effective.
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants