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 current npm broken. Remove babel-polyfill import #23

Closed
wants to merge 2 commits into from

Conversation

bySabi
Copy link
Contributor

@bySabi bySabi commented Mar 18, 2016

Hi @dennybritz

Current npm 0.2.4 is broken. This PR fix that.

I changed Object.assign to jQuery.extend. Now is more IE9 and mobile compliant without babel-polyfill dependencies.

@dennybritz
Copy link
Owner

I don't understand, why is it broken? I'd really not rely on jQuery.

@bySabi
Copy link
Contributor Author

bySabi commented Mar 18, 2016

Cause this line: https://github.com/dennybritz/neal-react/blob/master/js/app.js#L1 it broke if babel-polyfill npm is unmet. Adding the whole polyfill only for Object.assign IMO is not adecuate.

This package already use jQuery: https://github.com/dennybritz/neal-react/blob/master/js/components/vendor/stripe.jsx#L14. jQuery is needed for Bootstrap 4 module.

@bySabi
Copy link
Contributor Author

bySabi commented Mar 18, 2016

Hi @dennybritz do you already see the problem?

@dennybritz
Copy link
Owner

I decided to keep babel-polyfill in. It's true that it's currently not used beyond Object.assign but I think it may be useful in the future when adding new features. I moved it back into the dependencies.

I'd love to get rid of the jQuery dependency someday. Not everyone needs the Bootstrap Javascript.

@dennybritz dennybritz closed this Mar 22, 2016
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.

None yet

2 participants