Try out a change to CommonJS module resolution?

[Martin Probst]

Hi,

I have a PR out on github that substantially refactors how module loading works. The goal is to make CommonJS and ES6 module loading more flexible by supporting multiple module roots, and also to remove some cruft in the code.

I think it should not change any semantics nor break existing code (and the tests support my hypothesis ;-)), but I'd be very happy if somebody could patch it in and try it out in their existing code base.

So any CommonJS volunteers would be greatly appreciated!

Thanks,

Martin

[Maria Neise]

Hi Martin,

I've tried out your changes. We are currently adding CommonJS, AMD and ECMAScript 2015 module support to ClojureScript and are depending on the functionality of the Google Closure Compiler to convert JS modules to Google Closure modules. Instead of going through the compiler, we are calling the responsible classes (ProcessCommonJSModules, ProcessEs6Modules, ES6ModuleLoader) directly.

Overall it seems to work well. So far I have noticed two things which could cause some problems for us.

We have found the method toModuleName to be quite useful. Would it be possible to keep that one public, instead of making it package-private?

The other issue is about resolving CommonJS requires. As far as I understand, the changes you've made only rewrite a CommonJS require, if the required module was also given as a compiler input. Would it be possible to change this behaviour and check if the required module can be resolved against the current module root, in case it is not given as a compiler input? This would help us, since we are currently processing one module at a time, meaning that we are only passing one compiler input.

Kind regards,

Maria

[Martin Probst]

Hi Maria,

thanks for the feedback!

Regarding the public toModuleName, no problem - but I think I should still move it to Es6ModuleLoader, right? Having it in two places really doesn't help. Note though that with the multiple base URIs, it might not be entirely trivial to figure out what a URI should be relative to.

Regarding the individual file loading, that's a bit more complicated. I think overall JSCompiler is not currently built to discover files as it is parsing sources, the code base overall expects the entire set of transitive sources to be available, e.g. for type checking. The code so far intended to fail if a module is missing/cannot be found, and just didn't do that because of a bug.

Can you explain your use case in a bit more detail?

Martin

--

---
You received this message because you are subscribed to the Google Groups "Closure Compiler Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to closure-compiler-d...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/closure-compiler-discuss/fc978160-abb7-401d-a05f-6116e4283b5c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Martin Probst]

I've thought a bit about this. I think for the time being and for your use case, it'd be best to transpile the one file at a time, but provide all the other files needed as fake, empty stubs. That way the compiler knows what file imports are legal and how to handle them (e.g. append ./index.js where needed), but you still traverse file by file. Does that make sense?

Martin

[Maria Neise]

Sorry for the delay, I have been travelling ;)

I spent some more time on this, and I think we should be able to instantiate the ES6ModuleLoader with all the required compiler inputs, but then still convert one module at a time as shown in this small example [1]. So no need to change any logic in the ES6ModuleLoader.

It would still be better for us though, if a CommonJS require or an ECMAScript 2015 import would be translated properly to a goog.require even if a loadAddress for it cannot be found. The reason for this is that we are parsing the goog.require statements from a Google Closure module to get the dependencies of a module. We can then use this information to emit warnings if a dependency cannot be found or make sure we are passing all required files to the Google Closure compiler for optimisation of the final JavaScript code. Additionally, I think it would be nicer to have a JavaScript file that has syntax for either CommonJS or Google Closure dependency management, and not mix both. But that is just a personal preference ;) Would this change make sense to you or would it cause any problems?

Also, moving the toModuleName method to the ES6ModuleLoader is perfectly fine ;)

[1] https://gist.github.com/MNeise/ddec607ef0d1f6b7b590

[Martin Probst]

I spent some more time on this, and I think we should be able to instantiate the ES6ModuleLoader with all the required compiler inputs, but then still convert one module at a time as shown in this small example [1]. So no need to change any logic in the ES6ModuleLoader.

Good to hear!

 

It would still be better for us though, if a CommonJS require or an ECMAScript 2015 import would be translated properly to a goog.require even if a loadAddress for it cannot be found. The reason for this is that we are parsing the goog.require statements from a Google Closure module to get the dependencies of a module.

Just so I understand this correctly - do you have a multiple step process going on here, first from ES6 to goog.require style, then do something with the goog.require stuff?

 

We can then use this information to emit warnings if a dependency cannot be found or make sure we are passing all required files to the Google Closure compiler for optimisation of the final JavaScript code.

My experience is that it's usually better to explicitly specify all inputs to a compiler upfront, instead of having the compiler hunt around the file system. I presume you have a limited set of locations where you'd look for inputs in the first place, so maybe you can just glob those files together?

 

Additionally, I think it would be nicer to have a JavaScript file that has syntax for either CommonJS or Google Closure dependency management, and not mix both. But that is just a personal preference ;)

Absolutely - but that might be a separate issue, making ES6 imports work with goog.provide'd symbols and vice-versa, thus making it possible to have a code base with only one of the two.

 

Would this change make sense to you or would it cause any problems?

So we need to know all the inputs to decide whether a reference to "foo/bar" is actually a reference to "foo/bar/index.js".

Martin

[Martin Probst]

... hit send too early.

So we need to know all the inputs to decide whether a reference to "foo/bar" is actually a reference to "foo/bar/index.js".

... given that, I think we do need to know all inputs at the time when the process runs. We could look around the file system, but the compiler architecture is generally designed such that at this point, only CompilerInput objects should be floating around. We could try to build an abstraction for the inputs and a search method, but OTOH as written above, you might be able to just pass everything in in the first place. Would that work for you?

Martin

[Maria Neise]

Sorry, for not being clear enough. Yes, it is a multiple step process. We are first converting CommonJS, AMD and ECMAScript 6 libraries that have been included by the user to Google Closure modules and then later pass all sources in a single step to the Google Closure compiler for optimization. In order to figure out which sources to pass to the Google Closure compiler for optimization, we are creating a data structure for each JavaScript library, with information such as if it is a Google Closure library, which namespaces it provides, which other modules it requires, etc. To get some of those information from a Google Closure module, we are parsing the goog.require and goog.provide statements.

I think I figured out, where I went wrong. If you don't have the loadAddress of the required module, you cannot really rewrite the require statement, since you cannot get the new module name without the loadAddress.  Would it be an option to create the module name from the string given in the require statement in case the loadAddress cannot be found? But that would be quite similar to the previous approach then, right?

But overall, I am happy to try to pass all known sources to the compiler as inputs when converting a JavaScript module ;)

[Martin Probst]

I think I figured out, where I went wrong. If you don't have the loadAddress of the required module, you cannot really rewrite the require statement, since you cannot get the new module name without the loadAddress.  Would it be an option to create the module name from the string given in the require statement in case the loadAddress cannot be found? But that would be quite similar to the previous approach then, right?

For ES6, that'd work. But for CommonJS, there's the question of ./index.js, which we cannot answer unless we have knowledge of what sources are available.

Maritn 

[Maria Neise]

Ok, got it. Thank you for bearing with me and explaining everything ;)

[Martin Probst]

No worries, it took me a while to understand what ES6 module does, too :-)