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

pnotify v5 not ES5 compliant #389

Closed
h3llrais3r opened this issue Apr 23, 2020 · 5 comments
Closed

pnotify v5 not ES5 compliant #389

h3llrais3r opened this issue Apr 23, 2020 · 5 comments
Assignees

Comments

@h3llrais3r
Copy link

h3llrais3r commented Apr 23, 2020

Hi, first of all, thanks for this great package.
I'm trying to upgrade from from v4 to v5, and I'm still using ES5 syntax.
I noticed that after upgrading to v5, the iife is no longer available, so I'm using the PNotify.js directly from the /core/dist folder.
However, I noticed that this javascript file contains const variables, which are not ES5 compliant.
Is this as intended? Or can you also provide a file that is plain ES5, like it was before in /dist/iife folder?

The ES6 syntax breaks the backwards compatibility with older browsers like IE.

Of course, I can transpile myself to ES5 syntax, but I still wanted to share it here, just to know if it was intended or not. 😉

@hperrin
Copy link
Member

hperrin commented Apr 24, 2020

Hi @h3llrais3r,

Yes, this is a bug. It looks like the webpack config was set up wrong, and the code wasn't getting run through Babel. I'll fix this for the next version.

@hperrin
Copy link
Member

hperrin commented Apr 24, 2020

Even with the fix though, I don't know if it will compile entirely down to ES5. It should work in relatively old browsers though. Here's the browser list:

~/r/pnotify (develop|✚13…) $ npx browserslist
npx: installed 11 in 4.086s
and_chr 80
and_ff 68
and_qq 1.2
and_uc 12.12
android 80
baidu 7.12
chrome 81
chrome 80
chrome 79
edge 80
edge 79
edge 18
firefox 75
firefox 74
firefox 73
firefox 68
ie 11
ios_saf 13.3
ios_saf 13.2
ios_saf 12.2-12.4
kaios 2.5
op_mini all
op_mob 46
opera 67
opera 66
safari 13
safari 12.1
samsung 11.1
samsung 10.1

@h3llrais3r
Copy link
Author

h3llrais3r commented Apr 27, 2020

@hperrin Thanks for feedback.
Actually I found the incompatibility while uglyfying my code.
I'm using gulp to pack all vendor javascript code and I uglify it afterwards.
Because the code of v5 contains some const variables, gulp-uglify fails because that's es6 syntax, which is not supported by gulp-uglify.
So it would be nice if I got again ES5 compatible code. 😉

@hperrin
Copy link
Member

hperrin commented Apr 27, 2020

@h3llrais3r I just released v5.1.1. This should be compatible with your build process. If not, you can open another issue, and I'll fix it.

@h3llrais3r
Copy link
Author

@hperrin I confirm it's ES5 compatible and gulp-uglify does not complain about it anymore.
Thanks!

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

No branches or pull requests

2 participants