Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Does closure-compiler-js source map compatible with Rollup? #25

Closed
camelaissani opened this issue Sep 21, 2016 · 10 comments
Closed

Does closure-compiler-js source map compatible with Rollup? #25

camelaissani opened this issue Sep 21, 2016 · 10 comments

Comments

@camelaissani
Copy link
Contributor

I try to write a Rollup plugin with closure-compiler-js: rollup-plugin-closure-compiler-js

It seems that source map generated by closure-compiler is not compatible with Rollup.
I reproduce the issue in an unit test
Here the link to the CI build

Should I transform the source map to make it compatible with Rollup? Or do I need to set specific options here?

@camelaissani camelaissani changed the title Does closure-compiler-js source map compatible withRollup? Does closure-compiler-js source map compatible with Rollup? Sep 21, 2016
@Dominator008
Copy link

ccing @ChadKillingsworth in case he knows off the top of his head :)

@ChadKillingsworth
Copy link
Contributor

I'm guessing this is due to the fact that prior to this last month, Closure-compiler did not compose source maps. Changes are now merged to allow that, but it requires passing input sourcemaps to the compiler.

For the Java version gulp plugin, I compose the sourcemap manually after compilation. See https://github.com/ChadKillingsworth/closure-compiler-npm/blob/c03707e59fb7da90ae910da7c02ea68b4030714f/lib/gulp/index.js#L177-L193

Other reasons might be due to the paths or the file field of the source map not being correct.

On a related note, I'd love for you to add your rollup plugin directly to the java version at https://github.com/ChadKillingsworth/closure-compiler-npm. I would definitely review and approve such a PR. I've found that developers wish to manage the compiler version directly, so it makes npm version management so much easier if the plugins are hosted in the same repo.

@camelaissani
Copy link
Contributor Author

@ChadKillingsworth @Dominator008 thanks for your help. I'm gonna digging into your code to understand the source maps composition.

Once I've fixed the issue I would be happy to contribute to your project!

@camelaissani
Copy link
Contributor Author

I think closure-compiler works correctly. I would say that Rollup has a problem, but I am not 100% sure.
Here the issue open to the Rollup project: rollup/rollup#961
there are all the details in my last comment.

@ChadKillingsworth
Copy link
Contributor

Looking at the stack trace, it seems to be caused due to the fact that the source map sources array has no valid source files rollup can locate. My guess is that Input_0 should be the filename that rollup expects.

@samthor Thoughts on specifying the input filename for source maps? It could always be done post-processing too.

@samthor
Copy link
Contributor

samthor commented Sep 26, 2016

I'm on my phone so I don't have the whole context. However, the generated
source map has no name: it has a blank filename, since the generated source
is also unnamed and (right now) always a single file. It should be
trivially renamable. We could do it inside the compiler at some point.

On Mon., 26 Sep. 2016, 14:11 Chad Killingsworth, [email protected]
wrote:

Looking at the stack trace, it seems to be caused due to the fact that the
source map sources array has no valid source files rollup can locate. My
guess is that Input_0 should be the filename that rollup expects.

@samthor https://github.com/samthor Thoughts on specifying the input
filename for source maps? It could always be done post-processing too.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHRkLeQ7OTvPOixpQtaxHwSnvG0QdLsks5qt7ZpgaJpZM4KDTwZ
.

@ChadKillingsworth
Copy link
Contributor

@samthor The issue is the input filename for the sources array - that's a little bit harder (though still doable):

"sources":[" [synthetic:base] "," [synthetic:util/global] ","Input_0"]

@camelaissani
Copy link
Contributor Author

@ChadKillingsworth @samthor So far, in my tests, only es6 getters have produced this error in Rollup.
I would say there is also another problem?

@ChadKillingsworth
Copy link
Contributor

I'm guessing it's caused due to the synthetic sources. You can try post processing the source map - specifically fill in the correct filename for the source Input_0 and try setting the synthetic sources to null.

I believe the issue comes from https://github.com/rollup/rollup/blob/c5d9d347721ad3d168b2ca6df27894be69815d1c/src/utils/collapseSourcemaps.js#L90

@ChadKillingsworth
Copy link
Contributor

I'd like to replace the [ synthetic source map references with some sort of real file path. But that issue needs addressed at https://github.com/google/closure-compiler

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

No branches or pull requests

4 participants