Import Assertions potential changes: request for feedback

[GeoffreyBooth]

Hi @nodejs/collaborators. In the last TC39 meeting, the committee voted to downgrade the Import Assertions proposal from Stage 3 to Stage 2. This is the proposal that permits syntax like import data from './data.json' assert { type: 'json' } that used to live behind --experimental-json-modules and was unflagged in January 2022. Because of the possible change in direction of the import assertions syntax, the Node TSC is considering requiring the flag once again for this syntax starting in Node 20.0.0 this April; unless we can get some assurance from TC39 that the syntax won’t change.

How do you feel about the potential reintroduction of this flag? How disruptive would restoring the flag be, and how disruptive would potential future import assertions syntax changes be?

(Locking this thread just to collaborators, so that we can have a discussion without involving TC39 members. Once this thread has run its course I’ll post something on a TC39 repo.) cc @nodejs/modules @nodejs/loaders

[vdeturckheim]

How do you feel about the potential reintroduction of this flag?

I think it looks ok to me as long as we have the proper error message when the flag is not present and such import happens

how disruptive would potential future import assertions syntax changes be?

If we do it in a few release cycles, it should be ok? we need to document a clear migration path to the new syntax however

[VoltrexKeyva]

I feel like putting it behind a flag again is a good idea since not much users seem to be using import assertions from what I've seen, but regardless I think we should put out a warning for those who already use import assertions so that way the users can be aware of this breaking change.

About the syntax change, I agree with @vdeturckheim, we should document a good migration path to the new syntax.

However if we put this behind a flag again, we shouldn't unflag it until the authors of the import assertions proposal or TC39 decides that the syntax is not going to change again.

[GeoffreyBooth]

Just to be clear, there isn’t any proposed new syntax yet; and one of the options TC39 is considering is not changing the syntax, just the semantics of what happens as a result of the syntax. (In particular, they would relax a restriction that the assertion can’t influence the returned module, which doesn’t really apply to Node and is more of a browser/server thing.)

[anonrig]

Without the assurance from TC39, I agree that putting it behind an experimental flag is the right thing to do.

[bnb]

IMO +1 to moving it behind a flag again. I'd prefer this to not be a situation where the runtime currently does one thing and the spec/other runtimes do another thing, creating a situation where we have to cope with a Node.js wart for the next decade. Ripping the band-aid and having early adopters adapt now is IMO the far, far, far better path.

[ShogunPanda]

Same here +1 on flagging it again. We have to continue our effort to be as much spec compliant as we possibly can.

[marco-ippolito]

+1 to moving it behind a flag again

[mcollina]

+1 to move it behind a flag. I don't think TC39 can assure us that syntax would not change since they moved the proposal to stage 2.

[MylesBorins]

+1 to flag

[ruyadorno]

While I agree with the spec compliance sentiment in this discussion, I just wanted to make sure we're aware of the potential disruption to the ecosystem of user packages since all it takes is a single transitive dependency relying on import assertions and we have an enormous potential to get some very frustrated users (many of them confused about why they can't upgrade to node20 without breaking their tools and without a clue on how to fix it).

Also, I believe I am missing an important context in this discussion on why the experimental flag was removed while the feature was still on stage 3 in the first place.

Sorry I'm not the best at playing devil's advocate but I feel like the current discussion is too one sided at the moment so I'd love to see a few more arguments from the folks that want to reintroduce the flag since in my point of view this has the potential to introduce a lot of unnecessary churn to the ecosystem.

[guybedford]

The main concern here really is if the syntax changes from assert to a new keyword like with or if the way that json is imported changes considerably. This has been asked for by some, but it does not currently have consensus, and it is the V8 position that the assert syntax should be kept.

In either case though, the assert syntax will be staying, because of Chrome's usage itself V8 has no plans to remove the assert syntax. So it would be treated something like a compat path alongside any new syntax if there were a new syntax. It could possibly even be partially specified somewhere as a legacy import syntax say in Annex B. The deprecation path would be expected to be gradual.

For this reason I'd advise against flagging for now as well, and to reevaluate in due course.

[GeoffreyBooth]

In either case though, the assert syntax will be staying

That was the question asked in tc39/proposal-import-attributes#127 (comment). Could we get some kind of official statement from TC39 confirming that the current syntax won’t go away? Even if some new syntax appears in addition to it; if the old syntax never stops working, then we won’t need to reintroduce the flag.

Also, I believe I am missing an important context in this discussion on why the experimental flag was removed while the feature was still on stage 3 in the first place.

The feature itself is still experimental. The flag was removed because it seemed stable as far as we could tell, and the purpose of Stage 3 is for runtimes to implement a feature and gather feedback from as many users as possible. The feature was also unflagged in Chrome.

[aduh95]

if the old syntax never stops working, then we won’t need to reintroduce the flag.

Re-flagging the feature would be the reasonable thing to do, at least until the syntax is graduated to stage 3 again, because beside the syntax, the semantics might still change.

The feature was also unflagged in Chrome.

It's also unflagged on Deno. Side note: on V8 itself, it's still flagged.

[bnb]

While I agree with the spec compliance sentiment in this discussion, I just wanted to make sure we're aware of the potential disruption to the ecosystem of user packages since all it takes is a single transitive dependency relying on import assertions and we have an enormous potential to get some very frustrated users (many of them confused about why they can't upgrade to node20 without breaking their tools and without a clue on how to fix it).

Also, I believe I am missing an important context in this discussion on why the experimental flag was removed while the feature was still on stage 3 in the first place.

Sorry I'm not the best at playing devil's advocate but I feel like the current discussion is too one sided at the moment so I'd love to see a few more arguments from the folks that want to reintroduce the flag since in my point of view this has the potential to introduce a lot of unnecessary churn to the ecosystem.

This is a reasonable thing to flag, @ruyadorno <3

IMO the tradeoff here is "reflag now and break a small number of packages that are early adopters" vs. "have syntax that will almost certainly be broken, and have to force the ecosystem to cope with a more aggressive breaking change down the road". For me, knowing how much pain not ripping the band-aid off causes the ecosystem, it's preferable to break ASAP rather than letting a lot of debt build up and have a bigger, more impactful, more challenging break down the road.

[aduh95]

To add to what @bnb said, I think it's worth noting that using JSON modules implies that a runtime warning is emitted, and JSON modules are the only practical reason for using import assertions in Node.js atm. So if an app is using a dependency that relies on JSON modules, they are way aware because of the runtime warning – unless they are using --no-warnings, or the dependency is spawning a new Node.js process that it self relies on import assertions, and there's not much we can do for them, I think it's fait to assume that it would affect a very limited number of users. It's theoretically possible that there is code out there that uses import assertions syntax without using JSON modules of course, but imo quite unlikely, and even then breaking this in a semver-major is fair-game I think.

[GeoffreyBooth]

Reposting tc39/proposal-import-attributes#127 (comment):

@aduh95 I will present #131 in the March meeting. While the preference of the champions is to migrate from assert to with, there are different possible final outcomes:

• we decide to stick with only assert

• we have both assert and with for a while (1 year? 3 years? 🤷) to give web developers the time to migrate. JS code using assert has been written recently, so there is an high chance that it’s still maintained. At some point, the usage of assert will have decreased enough that it's considered compatible to completely remove it.

  • For example, Node.js could start warning in 20.0.0 unless the flag is specified, throwing in 21.0.0 unless the flag is specified, and remove the flag in 22.0.0.

• we end up having both assert and with forever

(I can’t post this in nodejs/node#46830, feel free to copy it if you want!)

[JakobJingleheimer]

+1 to flag

[MylesBorins]

We can always ship the flag in Node.js 20 and revisit / revert if it proves to be widely breaking. This is evidence we could bring to TC39 to advocate for not removing the syntax. If we flag it again without too many issues that similarly is evidence that perhaps the syntax can be removed.

[aduh95]

The proposals were promoted back to stage 3 with a new syntax, I think this can be closed now.

Folks, please update to the new syntax if you were using the old one.