node icon indicating copy to clipboard operation
node copied to clipboard

Re-flag `assert` syntax for import attributes in Node.js 22

Open nicolo-ribaudo opened this issue 1 year ago • 6 comments

What is the problem this feature will solve?

The assert syntax has been replaced by with, and it's currently only shipped by Chrome and Node.js. Other browsers will only ship the with syntax.

Chrome will ship with unflagged in 123, and is planning to remove support for assert in 126: https://v8.dev/features/import-attributes#deprecation-and-eventual-removal-of-assert. According to https://chromiumdash.appspot.com/schedule, Chrome 126 will be released in May.

What is the feature you are proposing to solve the problem?

One year ago in https://github.com/nodejs/node/issues/46830 there was a lot of support from TSC members to re-flag this syntax, but then nothing has been done. Node.js 22, which will be released in April, would be a good release to put assert back behind a flag.

What alternatives have you considered?

[node: I'm opening a new issue rather than commenting in #46830 because it was meant to be a place of discussion for Node.js collaborators, and as such it's locked]

nicolo-ribaudo avatar Jan 31 '24 17:01 nicolo-ribaudo

One year ago in #46830 there was a lot of support from TSC members to re-flag this syntax

To be clear, https://github.com/nodejs/node/issues/46830 is about re-flagging the JSON modules implementation (which was downgraded from stage 3 to stage 2 by TC39 when that issue was opened), not the assert syntax (which is almost the same thing, because JSON modules are the only supported use case for import assertions/attributes atm). Because the TC39 went back to stage 3, nothing was re-flagged. Support for the newer syntax (using with) has since landed on 20.x+, and the current plan is to keep the assert syntax around until v18.x reaches EOL – the last non-EOL version of Node.js which currently does not support the newer syntax.

A few things to consider:

  • If we introduced a new flag, it likely won't be backported to v18.x, which is in maintenance mode.
  • We don't want folks to use harmony V8 flags.
  • We could consider re-introduce the --experimental-json-modules to gatekeep the support for assert, and folks who want to support both Node.js 18.x and 22.x would need to pass that flag. Technically very easy to implement, however a bit hard to document.
  • Node.js 18.x EOL is scheduled for 2025-04-30 (circa Node.js 24.x release date).
  • If V8 drops support for assert, we could still float a patch to keep supporting it, although we probably want to avoid that if we can.

Our options seem to be:

  1. Keep supporting assert syntax unflagged until Node.js 24.x (likely we need to float a V8 patch for Node.js 23.x)
  2. Drop support for assert in 22.x. Folks who use JSON modules and want to support both 18.x and 22.x must use a harmony flag. Likely it won't be possible for them to support both 18.x and 23.x.
  3. Put assert syntax support behind --experimental-json-modules. (we might still need to float a V8 patch for 23.x, or drop support for it).

@nodejs/tsc wdyt?

aduh95 avatar Jan 31 '24 19:01 aduh95

FWIW I'm open to landing https://github.com/nodejs/node/pull/51136 on v18.x if it makes it easier to migrate to the newer syntax. cc @nodejs/lts

richardlau avatar Jan 31 '24 19:01 richardlau

Just to be clear, I assume you’re proposing flagging assert syntax but not with syntax, correct? We shouldn’t be flagging the latter.

I don’t see much point in flagging something that is going to be removed. We should add a deprecation warning and then just remove it in a semver-major. I would even go so far as to aim to remove assert in 22 unless there’s some reason we need to wait for 23. It’s never graduated to stable, despite being unflagged, so I don’t think it needs a full deprecation cycle.

GeoffreyBooth avatar Jan 31 '24 20:01 GeoffreyBooth

I would suggest to just remove support for assert in the next semver major. I don't really see the point floating a V8 patch like this when the proposal itself has already shifted direction (I don't quite understand why we were unflagging it when V8 had not shipped it in the first place TBH).

joyeecheung avatar Jan 31 '24 21:01 joyeecheung

I think removing it in 22 is the best course of action.

mcollina avatar Feb 01 '24 08:02 mcollina

Adding the https://github.com/nodejs/node/labels/release-agenda label as my position would depend on the decision of the LTS team regarding https://github.com/nodejs/node/issues/51622#issuecomment-1919789221.

aduh95 avatar Feb 02 '24 10:02 aduh95

Hey just checking what's the current expectation on this, given that Node.js 22 is coming in just one month. I can open a PR to re-flag (or remove?) it, but maybe https://github.com/nodejs/node/pull/51631 should first be released in 21 and 20?

nicolo-ribaudo avatar Mar 15 '24 14:03 nicolo-ribaudo

I vote for removing assert in 22 and adding a warning for all previous lines. What do others think?

And with would be unflagged.

GeoffreyBooth avatar Mar 15 '24 16:03 GeoffreyBooth

Note that with is already unflagged in 20 and 21 (and in 18 right now it just doesn't exist, not even behind a flag).

nicolo-ribaudo avatar Mar 15 '24 19:03 nicolo-ribaudo

Note that with is already unflagged in 20 and 21 (and in 18 right now it just doesn't exist, not even behind a flag).

https://github.com/nodejs/node/issues/51622#issuecomment-1919789221

richardlau avatar Mar 15 '24 20:03 richardlau