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

Fix shadowed models variable #49

Merged
merged 1 commit into from
Nov 8, 2017
Merged

Fix shadowed models variable #49

merged 1 commit into from
Nov 8, 2017

Conversation

zbarbuto
Copy link
Contributor

@zbarbuto zbarbuto commented Nov 6, 2017

Models is re-declared on line 81 of route.js which was causing webpack's uglification/optimisation to throw errors in prod builds.

I have moved the first models to the buildGraph scope which fixes the issue as the conversions models was only ever used in that function.

Closes #50

@natew
Copy link

natew commented Nov 7, 2017

Aha! Just saw this too! https://github.com/Qix-/color-convert/issues/50

@PaitoAnderson
Copy link

Getting this in a production React Native build as well

@caub
Copy link

caub commented Nov 8, 2017

just rename the first one to MODELS, instead of moving it in the function

edit: nvm, it's just using Object.keys it's fine

@ai
Copy link

ai commented Nov 8, 2017

@Qix- can you please accept it? It is very important since UglifyES break production websites.

@Qix-
Copy link
Owner

Qix- commented Nov 8, 2017

Hey sorry for the delay - was afk for my birthday.

Good catch :) Thanks for the PR.

@Qix- Qix- merged commit 874bf56 into Qix-:master Nov 8, 2017
@Qix-
Copy link
Owner

Qix- commented Nov 8, 2017

Released as 1.9.1.

@natew
Copy link

natew commented Nov 8, 2017

Unfortunately color relies on ^1.8.2 which means it doesn't pick this up. Releasing as a patch to 1.8, or updating color would fix!

Edit: and thank you for fixing quickly.

@shanshanzhu
Copy link

@natew If you use yarn you can override this version in your package.json
just specify
'color-convert': '1.9.1'

And/or raise an issue in 'color'

@brunolemos
Copy link

brunolemos commented Nov 8, 2017

Unfortunately color relies on ^1.8.2 which means it doesn't pick this up

Created this PR https://github.com/Qix-/color/pull/133

@Qix-
Copy link
Owner

Qix- commented Nov 9, 2017

color was published as 2.0.1 with the updated package versions.

Thanks again :)

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 this pull request may close these issues.

Breaking Uglify
8 participants