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.