Skip to content
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

Allow compiling in WHITESPACE mode #33

Closed
wants to merge 1 commit into from
Closed

Conversation

ribrdb
Copy link

@ribrdb ribrdb commented Feb 12, 2019

We're using WHITESPACE instead of BUNDLE because bundle doesn't support source maps.
(That doesn't fix source maps for j2cl code, but at least source maps for non-j2cl code still works)

@gkdn
Copy link
Member

gkdn commented Feb 13, 2019

J2CL already suppresses this as file level:

Could you clarify why you need this here? Maybe it is your own file?

@niloc132
Copy link
Contributor

As a possible alternative, we made a small patch to BUNDLE mode which adds sourcemaps (example running at https://colinalworth.com/boxes-and-lines-j2cl-devmode/), since we found running closure in BUNDLE mode to be rather faster than WHITESPACE (no need to fully parse the AST, just look for dependencies). We have not kept this up to date, since upstream closure removed the PersistentInputStore feature, which we also used to allow for keeping unchanged files and their dependencies cached in closure, but could look at trying to rebase it if there was interest.

Vertispan/closure-compiler@0c394bf

@ribrdb
Copy link
Author

ribrdb commented Feb 14, 2019

I don't have the logs handy at the moment, but was definitely getting this error for j2cl's internal packages without this change. I'm not sure why the suppression inside the file wouldn't work.
@niloc132 if we can get sourcemaps working in bundle mode that sounds much better. Whitespace compilation is still pretty slow.

Copy link

@MrbrooksIsSilent MrbrooksIsSilent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data Monitoring

@gkdn
Copy link
Member

gkdn commented Mar 11, 2019

Reproduced the issue and filed #40

@gkdn gkdn closed this Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants