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

[JS] Fix broken IE11 support by transpiling spread syntax using babel #586

Merged
merged 8 commits into from
Mar 2, 2020
Merged

[JS] Fix broken IE11 support by transpiling spread syntax using babel #586

merged 8 commits into from
Mar 2, 2020

Conversation

ruudsnl
Copy link
Contributor

@ruudsnl ruudsnl commented Feb 4, 2020

1️⃣ Is this something that is wanted/needed? Did you create a feature-request issue first?
Yes! It fixes the issue of Livewire not initializing in both IE11 and Edge. The issue can be found here: #585

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
Nope

3️⃣ Does it include tests if possible? (Not a deal-breaker, just a nice-to-have)
Nope

4️⃣ Please include a thorough description of the feature/fix and reasons why it's useful.

  • Added the following Babel plugin to transpile spread syntax in object literals:
    https://babeljs.io/docs/en/babel-plugin-proposal-object-rest-spread which is not supported in IE11 and early versions of Edge
  • Furthermore, to prevent similar issues in the future I added IE11 as a target in the Babel config.
  • Rebuilt the assets, the compatible version can be found in the dist directory

I have tested this in Edge using Browserstack and it seems to work.

More info on spread syntax compatibility:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals

5️⃣ Thanks for contributing! 🙌
More than welcome, thanks for Livewire.

@ruudsnl ruudsnl changed the title Fix broken IE11 support by transpiling spread syntax using babel [JS] Fix broken IE11 support by transpiling spread syntax using babel Feb 4, 2020
@calebporzio
Copy link
Collaborator

Hey @ruudschuurmans, I pulled down the PR (thank you BTW), but all the tests are failing with:

image

Are you seeing the same thing?

@ruudsnl
Copy link
Contributor Author

ruudsnl commented Feb 6, 2020

Hey @calebporzio I will check this out tomorrow and follow up. Thanks for letting me know!

@ruudsnl
Copy link
Contributor Author

ruudsnl commented Feb 8, 2020

Fixed by adding node: current as babel target for the test environment. (more at: jestjs/jest#3126)

@calebporzio
Copy link
Collaborator

I can't seem to pull your branch down, did you do any rebasing that might have messed up the tree?

image

@ruudsnl
Copy link
Contributor Author

ruudsnl commented Feb 10, 2020

oof, you're right my rebase was not the best way to go. I'll clean up the branch later and ping you!

@calebporzio
Copy link
Collaborator

Any progress on this?

@ruudsnl
Copy link
Contributor Author

ruudsnl commented Feb 28, 2020

@calebporzio Fixed the rebase, now JS tests are working too for me!
Weird thing is, even though no PHP files have been changed in this PR, the PHP tests are failing in the Github CI?

@calebporzio calebporzio merged commit 6f1733b into livewire:master Mar 2, 2020
@calebporzio
Copy link
Collaborator

Thanks!

@lancepioch
Copy link
Contributor

@all-contributors please add @ruudschuurmans for maintenance

@allcontributors
Copy link
Contributor

@lancepioch

I've put up a pull request to add @ruudschuurmans! 🎉

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

3 participants