closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

Passthrough for dynamic import appears to have become impossible again

Open AshleyScirra opened this issue 2 years ago • 6 comments

In issue #3707 I asked for support for passthrough of dynamic import in Closure Compiler. The issue was closed as support was apparently added.

However it seems that in subsequent updates the design of the feature has changed and now it's no longer possible to passthrough dynamic imports. I'm looking for use of dynamic import in the input scripts to be emitted exactly as-is in the output script. I can't find any way to do that in v20220502. I think this may be an oversight in how later changes affected the feature.

Example in.js:

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

Command line: java -jar ./compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --formatting PRETTY_PRINT --language_in ECMASCRIPT_2021

Observed result:

in.js:3:7: ERROR - [JSC_INVALID_MODULE_PATH] Invalid module path "https://cdn.example.com/library.js" for resolution mode "BROWSER"
  3|    await import("https://cdn.example.com/library.js");
              ^

1 error(s), 0 warning(s)

Further if you try using import("./somethingelse.js"), the error says Failed to load module "./somethingelse.js".

How do I get Closure Compiler to pass it through unmodified? I've tried --allow_dynamic_import, but that does nothing, and appears to be redundant anyway since v20210505. Adding --dynamic_import_alias=myimport does not change the errors either (perhaps because Closure Compiler doesn't think it needs to transpile it if the output language supports it?)

Maybe I made some mistake but as it stands it looks like it was once possible, but has since become impossible again.

AshleyScirra avatar May 16 '22 17:05 AshleyScirra

Did you try to run this with a version where this was still working? (From your description I'd say it's probably v20210504).

lvelden avatar May 17 '22 12:05 lvelden

Here is what I've found with past releases:

v20210202

Added experimental support for allowing dynamic import expressions import('./path') to pass through the compiler unchanged. Enabling this requires supplying the command line option --allow_dynamic_import and also setting --output_language=ES_2020

java -jar ./v20210202/compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --language_in ECMASCRIPT_2020 --language_out ECMASCRIPT_2020 --allow_dynamic_import

This works and passes through the dynamic import. (Confusingly the release note is wrong, the command line flag is language_out, not output_language)

v20210406

Add --dynamic_import_alias option which instructs the compiler to replace dynamic import expressions with a function call using the specified name. This allows dynamic import expressions to be externally polyfilled when the output language level does not natively support them.

java -jar ./v20210406/compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --language_in ECMASCRIPT_2020 --language_out ECMASCRIPT_2020 --allow_dynamic_import

This works and passes through the dynamic import.

java -jar ./v20210406/compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --language_in ECMASCRIPT_2020 --language_out ECMASCRIPT_2020 --dynamic_import_alias=myimport

This results in an error Dynamic import expressions cannot be transpiled. (even though not trying to transpile)

v20210505

Allow dynamic import expressions by default

java -jar ./v20210505/compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --language_in ECMASCRIPT_2020 --language_out ECMASCRIPT_2020

This results in an error Invalid module path "https://cdn.example.com/library.js" for resolution mode "BROWSER". Adding --allow_dynamic_import does nothing.

v20220502 with the command java -jar ./v20220502/compiler.jar --js in.js --js_output_file out.js --compilation_level SIMPLE --language_in ECMASCRIPT_2021 --language_out ECMASCRIPT_2021 has the same error as v20210505. It also has the same error if you pass --dynamic_import_alias.

So it looks like v20210505 actually broke this when the intent was to enable it by default.

FWIW I don't really need --dynamic_import_alias, I just need the pass-through of dynamic imports that used to be supported with --allow_dynamic_import up to v20210406.

AshleyScirra avatar May 17 '22 14:05 AshleyScirra

OK, I think I see where the problem is.

https://github.com/google/closure-compiler/blob/fa09c835b819d78a78148c5b4115b05c29e55185/src/com/google/javascript/jscomp/FindModuleDependencies.java#L168-L178

This bit of code attempts to resolve the dynamically imported module path. Then, if that attempt returns null, it skips adding the import to the list that will need to be updated at the end of the compilation. The intention of this code is to just pass through unmodified any dynamic imports for paths that cannot be resolved.

Unfortunately, the method it is calling (well, down the chain a bit) doesn't know that it's not an error if the path cannot be resolved, so it reports an error, stopping compilation.

https://github.com/google/closure-compiler/blob/fa09c835b819d78a78148c5b4115b05c29e55185/src/com/google/javascript/jscomp/deps/BrowserModuleResolver.java#L50-L61

It appears that to get the behavior we want here, we need an alternative version of the resolveJsModule() method that doesn't report errors.

brad4d avatar May 27 '22 16:05 brad4d

Could someone who cares about this option submit a PR to fix it? I don't think I'm likely to find time for it.

brad4d avatar May 27 '22 16:05 brad4d

@ChadKillingsworth perhaps you have an interest in fixing this?

brad4d avatar May 27 '22 16:05 brad4d

A workaround I have seen that works for this problem: import('https://root.cern.ch/js/7.0.1/modules/main.mjs') does NOT work (ERROR - [JSC_INVALID_MODULE_PATH] Invalid module path "https://root.cern.ch/js/7.0.1/modules/main.mjs" for resolution mode "BROWSER")

but

var modpath = 'https://root.cern.ch/js/7.0.1/modules/main.mjs'
import(modpath)

DOES work.

(command line in both cases is java -jar ../../closure-compiler-v20211006.jar rundetail2-devel.js --js 'closure/closure/goog/**.js' --dependency_mode=PRUNE --entry_point=atlasdqm.rundetail2 --js_output_file=2testadv.js --compilation_level=ADVANCED --dynamic_import_alias=import)

ponyisi avatar Dec 23 '22 05:12 ponyisi