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

getEmojiFlag is broken in v2.6.0 #78

Closed
slaweet opened this issue Jan 11, 2021 · 12 comments · Fixed by #80
Closed

getEmojiFlag is broken in v2.6.0 #78

slaweet opened this issue Jan 11, 2021 · 12 comments · Fixed by #80
Labels

Comments

@slaweet
Copy link

slaweet commented Jan 11, 2021

What's the problem?

https://github.com/renovatebot/renovate just tried to update countries-list 2.5.6 -> 2.6.0 on my project. And suddenly all calls to getEmojiFlag fail with:

TypeError: Cannot read property 'encode' of undefined

I think the error is coming from this line

return ucs2.encode(
which means that
const { ucs2 } = require('punycode');
for whatever reason doesn't work anymore.

What's the expected behaviour?

getEmojiFlag in countries-list 2.6.0 works just like in countries-list 2.5.6

What's my context?

I'm using countries-list in a frontend project in VueJS.

import { getEmojiFlag } from 'countries-list';
// Inside a Vue JS component
getEmojiFlag(country)

Running it with standard https://cli.vuejs.org/ either in browser or in jest tests in node -v v12.7.0

@dmythro
Copy link
Member

dmythro commented Jan 11, 2021

Thanks for reporting, will check this out.
Tests on this export as well are green though.

@dmythro
Copy link
Member

dmythro commented Jan 11, 2021

Tests for Node v12.20.0 were passed and it has 3 tests on getEmojiFlag():
https://github.com/annexare/Countries/runs/1671769398?check_suite_focus=true

And tests use built dist/index.es5.min.js:
https://github.com/annexare/Countries/blob/master/dist/index.es5.min.test.js

So, could you please check again why it fails for you?

slaweet added a commit to slaweet/countries-list-error-reproduce that referenced this issue Jan 11, 2021
@slaweet
Copy link
Author

slaweet commented Jan 11, 2021

@dmythro thank you for such a quick response 😍. I see your tests pass. They also pass on my machine. I don't understand why does it fail in my project. So I tried to create a minimal reproduce that you can run as well.

I just created a repo with a basic https://cli.vuejs.org/ project: https://github.com/slaweet/countries-list-error-reproduce/ and this commit reproduces the error: slaweet/countries-list-error-reproduce@b6c97d4

Screenshot from 2021-01-11 10-05-21

@slaweet
Copy link
Author

slaweet commented Jan 11, 2021

I also created another branch https://github.com/slaweet/countries-list-error-reproduce/compare/working to show that with the previous version countries-list v2.5.6 it works
Screenshot from 2021-01-11 10-20-14

@dmythro
Copy link
Member

dmythro commented Jan 11, 2021

Thanks for the repo! I think I have an idea why it happens: probably updated libs do not embed punycode, but it's installed while running tests so it can be require'd. Probably Webpack/terser needs additional param now.

Just curious if an update to tests can cover that :)

@slaweet
Copy link
Author

slaweet commented Jan 11, 2021

Thanks for the repo! I think I have an idea why it happens: probably updated libs do not embed punycode, but it's installed while running tests so it can be require'd. Probably Webpack/terser needs additional param now.

Now it makes sense to me 🙂, so I opened a PR to fix it #79

Just curious if an update to tests can cover that :)

I think it's just a matter of following npm guidelines when adding dependencies/devDependencies 🤷‍♂️

@dmythro
Copy link
Member

dmythro commented Jan 11, 2021

Don't think it is the solution. I had punycode as dev dependency because bundled encode/decode into JS.
Need to fix the bundle, newer versions of webpack/terser plugin are weird, with "include" option it disabled the minification somehow, checking other options.

@dmythro
Copy link
Member

dmythro commented Jan 12, 2021

Hey @slaweet, could you please check the build from #80?
You should be able to install a module from the git branch es-build-update as it's public. I've switched to Rollup and updated build/tests, should be good to go now as I see.

slaweet added a commit to slaweet/countries-list-error-reproduce that referenced this issue Jan 12, 2021
@slaweet
Copy link
Author

slaweet commented Jan 12, 2021

Thank you @dmythro

Tested in slaweet/countries-list-error-reproduce@ab3cbe5

Looks good:
Screenshot from 2021-01-12 10-53-36

@dmythro
Copy link
Member

dmythro commented Jan 12, 2021

Thanks! Looks like we're good to go then.

@dmythro
Copy link
Member

dmythro commented Jan 12, 2021

@slaweet released as v2.6.1, thanks for the issue!

@dmythro
Copy link
Member

dmythro commented Jan 12, 2021

Better build/minify setup also helped with a smaller bundle size :)

  • v2.6.1: 56.2kB minified, 15.5kB min+gz
  • v2.6.0: 65.2kB minified, 16.9kB min+gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants