tc39 / proposal-import-attributes

Proposal for syntax to import ES modules with assertions
https://tc39.es/proposal-import-attributes/
Apache License 2.0
599 stars 26 forks source link

No need for NLTH with `with` keyword #136

Closed Josh-Cena closed 1 year ago

Josh-Cena commented 1 year ago

The current syntax still says:

import ImportClause FromClause [no LineTerminator here] WithClause ;

NLTH was necessary to avoid ASI and compatibility issues with the assert contextual keyword. However, with is already a keyword, and is unavailable in modules, so the following is always a syntax error currently:

import foo from "foo"
with (something)

Is it possible to get rid of NLTH before with, because line terminators are already allowed before from?

ljharb commented 1 year ago

I suppose we could, but why would that be an improvement?

Josh-Cena commented 1 year ago

For the spirit of Non sunt multiplicanda entia sine necessitate, I suppose. Every NLTH should exist to solve actual ASI hazards.

Furthermore, it's not hard to envision that some may want to format the statement as:

import aRidiculouslyLoooongName
from "a-ridiculously-loooong-specifier"
with { type: "json" };

Aligning the three keywords has some kind of visual appeal. It's better if the language can refrain from imposing formatting restrictions if unnecessary.

nicolo-ribaudo commented 1 year ago

I have never seem someone splitting their import declarations like that, because usually multi-line imports are with named imports and thus the newline is before }. However, the language generally tries to avoid NLTH unless they are absolutely necessary, and in this case it doesn't solve any parsing ambiguity.

coderaiser commented 1 year ago

I don't think it has much sense, there is benefits in languages like Python, where everybody writes code in a similar way (and says that this is a big advantage), the same goes to:

which tries to unify syntax in some standard similar looking code. I agree with @nicolo-ribaudo nobody writes import in a couple lines, and better to keep it one line. Anyways we already have a place to break line:

import x2 from 'y' with {
    type: 'json',
};

In a typical way to dynamic imports:

await import('y', {
    with {
        type: 'json'
    }
});

And that's definitely enough, I'm sure that NLTH made intentionally in this case.

Josh-Cena commented 1 year ago

I agree with @nicolo-ribaudo nobody writes import in a couple lines

What if you have a really long import specifier, such that the specifier by itself exceeds the print width? Being able to write it as

import foooooooooooooooooooooooooo from
"baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaar"
with { ... };

Would be nice. Of course that's me playing the devil's advocate, but I'm saying it shouldn't be the language to decide how to format users' code; we haven't done that and we shouldn't.

I'm sure that NLTH made intentionally in this case.

No it isn't. It's simply refactoring artifact from the assert era.

ljharb commented 1 year ago

I don't think that would be nice at all :-) but yes, I agree the language typically doesn't, unfortunately, make these kinds of stylistic calls.

nicolo-ribaudo commented 1 year ago

I will bring this up at the next TC39 meeting for consensus given the current stage-3-but-not-really-due-to-#137 status, but to be honest I expect this to be not controversial at all.

nicolo-ribaudo commented 1 year ago

This has been fixed in the ecma262 PR for the proposal: https://github.com/tc39/ecma262/pull/3057

mgaudet commented 1 year ago

Just an aside -- while this change makes sense looking at with, it is slightly irksome for implementations that will have to support both with and assert that they have different grammars.

I'd argue we should just keep the parsing the same for with and assert here to avoid having to special case parsing for implementers who will handle both.