Skip to content

Conversation

@aduh95
Copy link
Contributor

@aduh95aduh95 commented Feb 1, 2024

Not sure what the semverness of this would be, hopefully we can backport it to v18.x if #51136 can land there. Here's what the warning looks like:

$ ./node --input-type=module -e 'import "./test/fixtures/empty.js" assert{}'(node:94657) V8: file://…/[eval1]:1 'assert' is deprecated in import statements and support will be removed in 12.6; use 'with' instead(Use `node --trace-warnings ...` to show where the warning was created) $ ./node --input-type=module -e 'import "./test/fixtures/empty.js" with{}' $ ./node -e 'import("./test/fixtures/empty.js",{assert:{} })'(node:94696) V8: [eval]:1 'assert' is deprecated in import statements and support will be removed in 12.6; use 'with' instead(Use `node --trace-warnings ...` to show where the warning was created) $ ./node -e 'import("./test/fixtures/empty.js",{with:{} })'

Should we mutate the V8 warning to say "in a future version" instead of "in 12.6"?

Refs: #51622

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Feb 1, 2024
Original commit message: [import-attributes] Deprecate 'assert' for removal in 12.6 See https://groups.google.com/a/chromium.org/g/blink-dev/c/ZHvzLaJZRvo/m/FgNDBjrtBQAJ Bug: v8:10958 Change-Id: I4d21c9f7aad1024b198b4a1cdfb4792a011da464 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5055681 Reviewed-by: Rezvan Mahdavi Hezaveh <[email protected]> Auto-Submit: Shu-yu Guo <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/main@{#92044} Refs: v8/v8@ae5a4db
Original commit message: [import-attributes] Deprecate 'assert' for dynamic import as well Bug: v8:10958 Change-Id: I7847bdb5d2c79f057f4e1df99f8f5889788f09cb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5249778 Commit-Queue: Shu-yu Guo <[email protected]> Reviewed-by: Leszek Swirski <[email protected]> Cr-Commit-Position: refs/heads/main@{#92123} Refs: v8/v8@26fd1df
@aduh95aduh95force-pushed the assert-deprecation-runtime-warning branch from d56a4be to 545a983CompareFebruary 1, 2024 13:12
@debadree25
Copy link
Member

Should we mutate the V8 warning to say "in a future version" instead of "in 12.6"?

I think "in a future version" or "12.6 of V8" would be better to use if its ok to float a patch for this or maybe a upstream v8 patch to say "12.6 of V8"

@aduh95
Copy link
ContributorAuthor

/cc @nodejs/v8

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.needs-ciPRs that need a full CI run.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@aduh95@nodejs-github-bot@debadree25