-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
Compiling squint-cljs (via @nextjournal/clojure-mode) on 2.27.1 throws ICE #1170
Comments
2.27.1 included an upgrade in the Closure Compiler, which I guess if the problem here. I'll see if I can figure something out, or maybe downgrade the GCC version again. |
This is also breaking our build too. Same error, but on chart.js
|
One temporary fix is setting Still trying to figure out if I can maybe tweak this, so I don't have to do a rollback of the Closure Compiler version. |
FYI: I'm also getting that "failed to convert sources" error, but on a different package:
Full trace:
|
That |
Hmm react-hook-form works fine for me, even without that Wether this is suitable for production or not I cannot say. Not sure which kind of features the code is actually using and how far back you need to go with browser support. The problematic feature on the other two is well supported in all browsers, so that should be fine. Still best to test though. |
I just released version I could not figure out why this fails. Using the npm |
I just cut a new |
@thheller I just tried with the new version; it gives the same error for me I'm afraid (when using |
Any particular reason you are setting I'm really not keen on downgrading again since this version also fixes some other issues. |
The new version seems to work perfectly for us, thanks |
No not really. This is in development mode only. Not sure why it's there. What do you normally use for development? |
I'd set the same value for development as in production, which should correspond for the Browser version you are expected to support. It makes zero sense to only set it for development, ever to any value. If you have to support IE11, which noone should nowadays, then you need to set If you just want to support modern browsers don't set it to anything at all. The default is You can increase it higher if your know your audience is going to have up-to-date browsers, e.g. for internal UIs. I stick to the default, since it doesn't matter whatsoever for CLJS and is usually only used for JS deps that want to use cutting edge JS features that might be a bit too cutting edge for the average user. ES2020 is what I would consider safe and my condolences to anyone having to support lower. |
Thanks for your detailed answer Thomas! I just verified that the default now also works. A while back we needed to add the output features set because of some library IIRC, but apparently it's no longer needed. So all good now! :) |
Still worked on 2.26.7
repro: https://github.com/meeseekz/shadow-issue-squint
The text was updated successfully, but these errors were encountered: