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

rollup doesn't strip const with comma operator #1937

Closed
yuqianma opened this issue Feb 2, 2018 · 2 comments · Fixed by #1949
Closed

rollup doesn't strip const with comma operator #1937

yuqianma opened this issue Feb 2, 2018 · 2 comments · Fixed by #1949

Comments

@yuqianma
Copy link

yuqianma commented Feb 2, 2018

Check here: repl

I encounterd a problem and reproduce this minimal version.
You can see the line backHinge = BACK_HINGE_SIZE should be stripped but not. This leads to the "not defined error".

However if I write them in two line version, e.i.:

const backHinge = BACK_HINGE_SIZE;
const fontHinge = FONT_HINGE_SIZE;

Everything runs perfect.

Does it mean const & comma syntax is not supported? But since it's verbose to write multiple const, this feature is useful.

Anyway, thanks for your awesome work!

@lukastaegert
Copy link
Member

Hi @yuqianma,
thanks for filing this issue! To answer your question:
Yes, your use case should be handled properly by rollup and this is a bug.

Incidentally I am refactoring some core parts of the statement rendering logic at the moment where fixing this issue would fit in nicely. So there is a good chance this will be fixed in the near future :) Until then, though, you will need to split this specific constant.

For reference, I am quite sure this is related to #1831.

@yuqianma
Copy link
Author

yuqianma commented Feb 2, 2018

Hi @lukastaegert

Thanks! It's great to know you guys are striving for a better rollup. Can't wait to see the new version!

lukastaegert added a commit that referenced this issue Feb 6, 2018
lukastaegert added a commit that referenced this issue Feb 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants