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

Latest Update Breaks Safari #38

Closed
arist0tl3 opened this issue Sep 14, 2017 · 4 comments
Closed

Latest Update Breaks Safari #38

arist0tl3 opened this issue Sep 14, 2017 · 4 comments

Comments

@arist0tl3
Copy link
Contributor

Safari doesn't allow const declarations in use strict mode, which is causing a white screen when trying to use this package.

Rolling back to 2.10.2 resolved the issue. The line in question is below. Will submit a PR.

https://github.com/bobby-brennan/rss-parser/blob/5d290bd139f5f63ec463196b505027f8f92049e0/index.js#L215

@rbren
Copy link
Owner

rbren commented Sep 14, 2017

Thanks! Fix is published as 2.10.5.

Out of curiosity, how are you using this in-browser? dist/rss-parser.js should have const removed after going through webpack.

@rbren
Copy link
Owner

rbren commented Sep 14, 2017

Also, if you have any ideas on how best to test for this, let me know.

@arist0tl3
Copy link
Contributor Author

You are right! I'm using it in Meteor, in a method. Not sure if you know Meteor, but methods are made available to both client and server (so that the client can do optimistic updates). Things were working out of the box, so I didn't write any code to specifically limit the function to the server side. But then this latest update obviously caused a problem, as the client-side evaluation on safari was complaining.

What I'd normally do is wrap code like this in a server-only block if the client complains. But in 2.10.2 (where I started), things were working so I didn't think a wrap was necessary.

Let me know if you have any questions -- I think this is probably an edge case, as typically the client wouldn't be evaluating server-side code.

@rbren
Copy link
Owner

rbren commented Sep 15, 2017

Ahh that makes sense. I'd been considering rewriting index.js with ES6 features since dist/ files will be translated using babel, but it's good to know that it'd break some workflows. If I do go that route I'll release it as a major version so it won't break Meteor builds.

Thanks for reporting/fixing!

@rbren rbren closed this as completed Sep 15, 2017
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

No branches or pull requests

2 participants