closure-compiler

closure-compiler copied to clipboard

It would be great for Node module resolution to just work, without extra arguments.

Right now, you need --module_resolution NODE which does nothing unless --process_common_js_modules is also passed, and even then, it's shaky. I would confidently say this doesn't work in a way that's reasonably accessible (even though I have managed to get it to work in the past).

I am not sure what flag I need to tell the compiler to look in node_modules/, even when I manually pass in with --js. A tale in a few commands:

$ cat test.js

import { Widget } from 'web-widgets';
console.log(Widget);

Execute in Node without issues:

$ node test.js

[Function: e] {
  superClass_: {},
  exportStyles: [Function (anonymous)],
  from: [Function (anonymous)],
  styles: [Getter]
}

Check that we definitely have the module installed (hence why the code executes):

$ ls node_modules/web-widgets/

dist  LICENSE  package.json  README.md

Run compiler, complete with flag soup:

$ google-closure-compiler -O ADVANCED --process_common_js_modules --module_resolution NODE --entry_point test.js test.js

test.js:1: ERROR - [JSC_JS_MODULE_LOAD_WARNING] Failed to load module "web-widgets"
  1| import { Widget } from 'web-widgets';

????

I feel like this ticket has been opened a thousand times and it ends with @ChadKillingsworth saying you need to pass in node_modules/ as --js arguments as well as package.json. I have tried that, and it does not work. it also parses everything in node_modules/ and dumps a ton of errors. We do not want that. We just want to import the module we are importing.

If I can throw a source file at V8 without issues, I should be able to throw it at the Closure Compiler without issues as well. This is true for everything up until ES6 modules, where everything just breaks down.

Thank you everyone who maintains this tool, it is one of the most important technologies we have in improving the web and I apologize for my frustration. I would be happy to contribute regarding this issue.

I can get output by manually supplying the specific package I am importing with --js and then ignoring errors with --jscomp_off '*':

$ google-closure-compiler -O ADVANCED --process_common_js_modules --module_resolution NODE --js node_modules/web-widgets/**.cjs --js test.js --entry_point test.js --jscomp_off '*' --js_output_file out.js

/CC @ChadKillingsworth

Chad, can you take a look here?

I've also added this to the next sync meeting on open source support, we can take a look at what the current state is here.

There were a few different reasons for this design. I can outline them.

1. The compiler is designed to use with build systems such as Bazel which emphasize consistency and repeatability of builds. These systems require you to know the exact input file set up front.
2. The now-removed GWT version at the time did not have file system access.
3. The only other file that was automatically loaded from the file system in the compiler was source map sources.

Changing this would be non-trivial. It also is not something that would be enabled inside Google (see point 1). It also may not be the immediate win people want. The compiler would have to load these files directly meaning no pre-processing step could be utilized (Typescript transpilation for instance). You would frequently end up with scenarios where the source files for the project should not be loaded (because they would need some sort of pre-processing) and then the request would be to scope this auto-loading of files to node_modules folders only.

Part of the reason I don't personally encounter these problems is I use some sort of tool to instrument and invoke the compiler. It depends on the project but I use both gulp and webpack. In these cases, the compiler doesn't interact with the filesystem at all but the files are piped into the compiler via STDIN. I realized a couple of months ago that the flags to do that were not documented, but they alleviate the file path problems that occur from using the command line flags directly. The reason the command line flags don't just "work" is that the input file paths are not normalized. The following are all seen as different files and mean different things:

• --js src/path/file.js
• --js ./src/path/file.js
• --js /absoulte/folder/src/path/file.js

To solve both this and to properly calculate split points for the compiler --chunk code splitting flags, I wrote the Closure Calculate Chunks utility. That utility understands all 3 dependency systems (ES Modules, Common JS Modules and Goog Module/Closure Library dependencies).

If there is a desire to actually change the compiler to discover and late-load source files, that will be a challenging endeavor. I'm happy to assist in reviewing pull requests for that type of work, but I'm currently focusing my development efforts on other projects so couldn't be the one to actually implement the change.

Lastly on the --process_common_js_modules flag, it wasn't always as stable as it is now. While rewriting commonjs modules for static analysis has turned out to work very effectively now, not all commonjs modules are compatible with this. I would have no objections to enabling this by default in the open source command line runner now however.

Minor point of detail: you wrote "I can throw a source file at V8 without issues", but I think you meant "node" instead of "v8" there. I think part of the issue here is that node module resolution is its own algorithm, beyond what browsers, v8, and (maybe?) Closure compiler do. See e.g. https://nodejs.org/api/esm.html#esm_resolver_algorithm_specification for an example of what code you'd need to implement.

Another option would be to move the closure-calculate-chunks functionality into the npm packages directly. For node environments, it would effectively solve the same problem.

Very sorry for not responding sooner @ChadKillingsworth, thank you very much for your notes. I see that you have been actively working on that utility as well, I would say rolling it into the NPM packages would be really smart - I recommend people get the Compiler through NPM anyway, so they can just reference it with google-closure-compiler rather than dealing with Java.

I would be happy to help in any capacity, especially with regard to making this even more user-friendly - it seems like logically, we could create a single CLI (called google-closure-compiler-single-entry or something lazy) which would invoke the closure-calculate-chunks tool and then compile the outputs. The goal would effectively be for the output bundles to execute exactly like they would if we ran node my-single-entry.js.

Just offering the ecosystem a way to reliably Closure Compile single-entry ES modules would be a HUGE win, and it looks like you've done most of the hard work already.

To roll the closure-calculate-chunks project into the main npm repo, it's going to need tests and support loading through some sort of adapter so it can use in-memory translated files such as those used from gulp.

This is probably going to be the focus of my future contributions Chad, is learning your calculate-closure-chunks tool and rolling it in / writing tests. I have still not read your notes on that tool yet.

That layer might be a good place to address a Tsickle/Closure Compiler issue related to NodeJS builtins as externs as well.

I've long wanted to just be able to use typescript d.ts files as externs with no additional work.

I am a little confusion about this strange behavior. If I use java -jar compiler.jar --js src\a.js --js "D:\myweb\node_modules\js-base64\package.json" --js "D:\myweb\node_modules\js-base64\base64.mjs" --compilation_level ADVANCED --module_resolution NODE --process_common_js_modules to compile the code, I will get Failed to load module "js-base64". If I use releative path to replace absolute path (for example: java -jar compiler.jar --js src\a.js --js "node_modules\js-base64\package.json" --js "node_modules\js-base64\base64.mjs" --compilation_level ADVANCED --module_resolution NODE --process_common_js_modules), it works fine. I am sorry that my English is not well.