Skip to content

Support passthrough dynamic import, or basic code doesn't work #3707

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

Open
AshleyScirra opened this issue Oct 19, 2020 · 14 comments
Open

Support passthrough dynamic import, or basic code doesn't work #3707

AshleyScirra opened this issue Oct 19, 2020 · 14 comments

Comments

@AshleyScirra
Copy link

Please at least support a basic passthrough for dynamic import for Closure Compiler. In the current version, any presence of dynamic import at all causes a compilation failure. This causes even basic use cases of dynamic import to cause the entire compilation to fail.

Example in.js:

(async () =>
{
	await import("https://cdn.example.com/library.js");
	externalLibrary.someFunc();
})();

Command line:

java -jar ./closure-compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --formatting PRETTY_PRINT --language_in ECMASCRIPT_2020 --language_out ECMASCRIPT_2020

Observed result:

in.js:4:7: WARNING - [JSC_PARSE_ERROR] Parse error. This language feature is not currently supported by the compiler: Dynamic module import
  4|    await import("https://cdn.example.com/library.js");
              ^

ERROR - [JSC_FEATURES_NOT_SUPPORTED_BY_PASS] Attempted to run pass "markUntranspilableFeaturesAsRemoved" on input with features it does not support. Running pass anyway.
Unsupported features: [Dynamic module import]

1 error(s), 1 warning(s)

No output file is generated.

Expected result:

Just output the file with all transforms, but passthrough import(XYZ) -> import(XYZ)

This is blocking us using JavaScript modules at all in our code. We want to insert a dynamic import, but then Closure Compiler will refuse to process any of our code at all.

@AshleyScirra
Copy link
Author

The same goes for import.meta too - we also need to use import.meta.url, but the fact all compilation fails if it's present anywhere is a major limitation. That should be able to passthrough too. We know the url or other metadata might change after compilation; that's fine, it either changes nothing in our case or is something we deal with.

@concavelenz
Copy link
Contributor

We need some way to distinguish between imports that should be rewritten and those that folks want to preserve.

@AshleyScirra
Copy link
Author

Just want to link to this comment here as well - for the "just minify" case I think it would be fine to have a flag that just says "pass through all imports/modules features without changing them".

@ctjlewis
Copy link
Contributor

Yeah, it would be great if I could tell the compiler not to worry about import.meta. @supress or @external import.meta would be good approaches - tried both, compiler won't take the source if it contains an import.meta declaration.

@brad4d
Copy link
Contributor

brad4d commented Nov 3, 2020

Note that there is already an open issue for dynamic import support

#2770

I guess this one is asking for something a bit more targeted, but I'm still a bit inclined to close this as a duplicate so all the discussion will be in one place.

@AshleyScirra wdyt?

@AshleyScirra
Copy link
Author

AshleyScirra commented Nov 4, 2020

I think #2770 is asking for full support. I'm only asking here for Closure Compiler not to fail compilation due to the presence of dynamic import, i.e. the minimum possible support. I think there's basically three levels of support:

  1. Fail compilation if feature exists (current state)
  2. Ignore feature/pass through compilation without alteration (what I'm asking for here)
  3. Full support (e.g. potentially taking in to account dead code elimination, bundling, transpilation, tree shaking etc) - as apparently described in issue [FEATURE] Support dynamic import #2770

I'd have thought passthrough is more or less trivial, whereas full support would involve more significant work.

@brad4d
Copy link
Contributor

brad4d commented Nov 4, 2020

@AshleyScirra do you feel this issue would be completely closed by the following behavior?

If the compiler's output is set to at least ES_2020, then:

  1. import(anything) will be left in the source code entirely unmodified.
  2. Specifically, the compiler will not pay any attention at all to the value passed to import(), so it won't notice if it looks like the path to a file that is included in the compilation.
  3. The compiler will infer the type of all such expressions to be Promise<*>. (You must cast the result of the promise to some type the compiler knows about in order to do anything with it.)

@AshleyScirra
Copy link
Author

Yes, that sounds like it would do it.

@GreenFootballs
Copy link

Has this issue been addressed yet? I've had to revert to an earlier version to compile a project I'm working on, because dynamic import causes the compiler to exit with an error.

@ChadKillingsworth
Copy link
Collaborator

There is a PR for basic pass through support under review: #3706.

@GreenFootballs
Copy link

Great, thanks. I'll watch for it.

@RReverser
Copy link
Contributor

Looks like PR above addressed dynamic import, but import.meta still fails.

@ChadKillingsworth
Copy link
Collaborator

dynamic import and import.meta are now supported

@ChadKillingsworth
Copy link
Collaborator

There's a couple of more places that need to support import.meta.

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

No branches or pull requests

7 participants