search

tc39 / proposal-source-phase-imports

Proposal to enable importing modules at the source phase
https://tc39.es/proposal-source-phase-imports/
MIT License
129 stars 9 forks source link

Ban `import source from from` #43

Closed lucacasonato closed 1 year ago

lucacasonato commented 1 year ago

This commit bans import source from from by adding an early error for this syntax production.

Closes #42

ljharb commented 1 year ago

This is a feasible but also pretty hacky workaround here, and it's strange to me to make from a quasi-reserved word again.

lucacasonato commented 1 year ago

@ljharb The other option is to do nothing, in which case we incur an at most 3 token lookahead.

I don't have a very strong preference, but I want to note that if we ban this, we can always lift it later if it turns out the restriction was unwarranted.

ljharb commented 1 year ago

If the syntax changes to import.<phase> then it would be moot, yes?

guybedford commented 1 year ago

@ljharb that was only for the dynamic case - import.<phase>(expr). The static case is not changing, and that was clarified with Ron as well.

I don't personally have an issue with import source from from 'test' and import from from 'test' both being supported either.

ljharb commented 1 year ago

ah, i missed that. I have a mild preference for the static syntax to also be import.phase tbh. why can't that be an option?

guybedford commented 1 year ago

@ljharb I don't think we considering changes the fundamental static grammar at this stage of the proposal process. The dynamic case falls under the existing import.meta meta property convention and was justified by the TypeScript folks for some valid reasons. Whereas the language doesn't have any precedent for static grammar using . without it meaning a meta property or member expression.

ljharb commented 1 year ago

OK, but that still doesn't mean this arbitrary grammar error is an ideal solution :-/

guybedford commented 1 year ago

@ljharb would you prefer we revert this syntax error? Our thinking was to be cautious about this case, but if you have concerns about the error we can revert too.

ljharb commented 1 year ago

I'd like it to be discussed further before advancing with the change, but I don't think you need to revert it now. It's fair that this is an easier restriction to remove than to add later, though.

nicolo-ribaudo commented 1 year ago

Just for the record, this PR bans import source from from "x" but not import source fr\u{6f}m from "x". I think this is good, since the second is absolutely unambiguous. Obviously a linter should also ban the second, but it's not the spec's job to do it.