Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic import should accept multiple arguments #55

Open
Jack-Works opened this issue May 12, 2020 · 8 comments
Open

Dynamic import should accept multiple arguments #55

Jack-Works opened this issue May 12, 2020 · 8 comments

Comments

@Jack-Works
Copy link
Member

Current syntax:

ImportCall[Yield, Await]:
    import(AssignmentExpression[+In, ?Yield, ?Await])

Proposed syntax:

ImportCall[Yield, Await]:
    import Arguments[+In, ?Yield, ?Await]

Currently import("a", { }) is SyntaxError and it's harder for developers to do "feature detecting" in future. The module attribute proposal is going to add 2nd parameter for ImportCall but developers can't write code like this:

try {
    return await import("./a.js", { attribute: something })
} catch(e) {
    return import("./a.js")
}

Because it is a SyntaxError. IMO it's better to allow more than 1 parameter and throw a runtime error so it will be easier for the developers in the future.

Since the module attribute is just stage 1, make this change in that proposal will be too late. Too many versions of the browser will only accept the exact 1 argument and it's impossible to write the feature detect above.

It's better to extend this syntax asap for the future (and throw at runtime for now) and it won't break anything (because it just makes invalid syntax valid).

@WebReflection
Copy link

so, basically you would prefer this?

return await import("./a.js", { attribute: something }).catch(() => import("./a.js"));

it makes it less obvious that the import(...) supports or not arguments (the feature detection you mentioned) but since detecting dynamic import.length is also not an option, I guess it might make sense.

@Jack-Works
Copy link
Member Author

import.length is an interesting idea but it doesn't resolve the problem:

// if there is a import.length meta property
if (import.length > 2) import("a", {}) // oops, still SyntaxError. Can not run
else import("a")

@littledan
Copy link
Member

I don't think this makes sense to add separately from the module attributes proposal. Given that there are browsers shipping now without this syntax change, you'd need your feature test to work there too. Let's just continue discussion in the module attributes proposal.

@Jack-Works
Copy link
Member Author

This can be changed earlier than that proposal so in future it is less pain. (Less range of versions of browsers in this embarrassing situation)

@littledan
Copy link
Member

I don't think I agree that this is important to be able to feature test for. It is really unclear to me how/whether this would be usable in practice, even if it were supported. The typical usage pattern is with a static import, where it will be a syntax error in browsers without module attributes. I'd recommend people initially adopt module attributes similarly to async/await: through transpilers.

Can we move this discussion to the module attributes repo?

@Jack-Works
Copy link
Member Author

You can move it

@littledan littledan transferred this issue from tc39/ecma262 May 12, 2020
@littledan
Copy link
Member

So, this proposal lets import() take a second argument. It's unclear to me how we could get that second argument part shipped sooner than the rest of module attributes to avoid the compatibility problem @Jack-Works raises--the agreement on the second argument probably rests on coming to agreement that we should do the rest of this proposal.

@jespertheend
Copy link

I am running into this issue at the moment. Though it's not an issue for production builds.
We develop on Chrome without any build step through the use of import maps and constructable style sheets.
But in Safari these are not supported, so anytime we need to test anything in Safari we bundle everything and make a debug build of our site.
The problem is we use a dynamic import to reload all the style sheets to make development easier. Which isn't really necessary for the occasional debug build we make. But it causes builds to entirely fail in Safari because of the syntax error.

I reckon there's not much that can be done about this spec-wise though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants