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

compress option in terser inside production es rollup config produces invalid JS #36

Closed
swyxio opened this issue Apr 27, 2019 · 9 comments · Fixed by #58
Closed

compress option in terser inside production es rollup config produces invalid JS #36

swyxio opened this issue Apr 27, 2019 · 9 comments · Fixed by #58

Comments

@swyxio
Copy link
Collaborator

swyxio commented Apr 27, 2019

Current Behavior

here is my repo i'm working on https://github.com/sw-yx/react-netlify-identity-widget/tree/rollupProductionIssue

if you:

yarn build
yarn link
cd example
yarn && yarn link "react-netlify-identity"
yarn start

you will see this error:

image

however if you change the package.json to use the development versions:

image

it works!!

to me this indicates a rollup config issue.

Expected behavior

the production output should work the same as development output

Suggested solution(s)

i think this could be a terser bug. or a setting that is incompatible with React.

here is the offending unexpected token - prettified for easier reading

image

i'm not sure what rollup setting caused this but it definitely only exists in the production build.

Your environment

Software Version(s)
TSDX 0.3.4
TypeScript 3.4.4
Browser chrome
npm/Yarn yarn
Operating System osx
@swyxio
Copy link
Collaborator Author

swyxio commented Apr 27, 2019

i went and disabled compress inside the terser settings and that fixed it:

image

trying to figure out what makes it go wrong

@swyxio swyxio changed the title production es rollup config is doing something wrong :( compress option in terser inside production es rollup config produces invalid JS Apr 27, 2019
@TrySound
Copy link
Collaborator

@sw-yx Does it look similar to you?
terser/terser#333

@swyxio
Copy link
Collaborator Author

swyxio commented Apr 27, 2019

i saw your issue, yes it does look similar.

@jaredpalmer
Copy link
Owner

What should we do here since it is downstream?

@swyxio
Copy link
Collaborator Author

swyxio commented Apr 30, 2019

what's the size impact on you if you turn off compress? i dont have the option because my code plain doesnt work without it (and its not like i'm doing some crazy nonidiomatic React stuff). as far as i can see, thats the only option for now, and i pray @TrySound can figure it out.

We can also choose to document it as well as how to patch the package, for early adopters of TSDX who may run into this.

@TrySound
Copy link
Collaborator

TrySound commented Apr 30, 2019

To be honest terser code is a mess. I can't find a fix for that. You may turn off collapse_vars option. It should fix the problem.

@swyxio
Copy link
Collaborator Author

swyxio commented Apr 30, 2019

ohh i see. i thought there was no option available. thank you i will try that.

@jaredpalmer
Copy link
Owner

So what should we do for now?

@swyxio
Copy link
Collaborator Author

swyxio commented Apr 30, 2019

@jaredpalmer i have verified that Bogdan's solution works. i recommend adopting it in tsdx and have PR'ed accordingly

jaredpalmer pushed a commit that referenced this issue Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants