Skip to content

ES5 transpiling #254

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

Merged
merged 7 commits into from
Nov 8, 2019
Merged

ES5 transpiling #254

merged 7 commits into from
Nov 8, 2019

Conversation

sigfriedCub1990
Copy link
Contributor

@sigfriedCub1990 sigfriedCub1990 commented Oct 22, 2019

Issue #238: Older browsers support through ES5 transpiling and polyfills.

Changes

  • Updated ruby gems, specially webpacker to 4.0 version.
  • Updated .babelrc to use the newly version of babeljs and @babel/preset-env
  • Updated node packages to use latest babeljs, @babel/preset-env and necessary plugins.
  • Added core-js polyfills to webpack entry file.

Notes

  • Please, take special care with the first bullet point above, since I'm not a ruby developer probably I broke something 🙁, if that's the case I'll gladly downgrade the packages and I'll only upgrade the webpacker since that's the only critical package to upgrade.

@jonasjabari
Copy link
Member

Hey @sigfriedCub1990! Thanks for your work here so far. I actually would ask you not to change the gem versions and only focus on the javascript adjustments :) Just to limit the scope of this PR. We will update the gems in other specific PRs. Thanks!

@sigfriedCub1990
Copy link
Contributor Author

No problems @jonasjabari, the only gem I really need to update is the webpacker gem

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sigfriedCub1990 looks very good to me!

Just two things that should happen before we can merge - could you

  • Merge the latest develop branch into your repo
  • Delete your Gemfile.lock and only run bundle install? We'll do the upgrade stuff (Rails 6 et al.) in a separate branch :-)

@pascalwengerter
Copy link
Contributor

Hey @sigfriedCub1990 thank you for the clarifications!

The thing with Gemfile.lock didn't turn out the way I hoped, though. Could you simply remove the file from your PR and I recreate it locally after merging your PR? :)

@pascalwengerter
Copy link
Contributor

Hey @sigfriedCub1990, thank you for the fast reactions!

I've now checked out your changes locally and the tests suite passes, so we're fine on that side. In the vendor/assets/javascript/ however, there's still a number of const and let ES6 in the .js files that are created/updated by running ./bin/webpack in the builder/ directory.

Do you have an idea how to fix that? Or am I missing a step somewhere?

@sigfriedCub1990
Copy link
Contributor Author

Hey @sigfriedCub1990, thank you for the fast reactions!

I've now checked out your changes locally and the tests suite passes, so we're fine on that side. In the vendor/assets/javascript/ however, there's still a number of const and let ES6 in the .js files that are created/updated by running ./bin/webpack in the builder/ directory.

Do you have an idea how to fix that? Or am I missing a step somewhere?

Let me check that after lunch :)

@sigfriedCub1990
Copy link
Contributor Author

Hey @pascalwengerter, it seems like there is partial support in IE 11 for let and const declarations, so it shouldn't be a problem. More info here const, let.
Anyway, if we could test this with IE 11 users I would be more comfortable :) and if there's a problem I'll fix it.

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigfriedCub1990 great work so far! We'll test the changes using browserstack and will let you know if IE does complain 😉

@pascalwengerter pascalwengerter merged commit cee1c14 into matestack:develop Nov 8, 2019
@sigfriedCub1990 sigfriedCub1990 deleted the es5-transpiling branch January 21, 2020 14:16
@jonasjabari jonasjabari added this to the 0.7.3 milestone Feb 10, 2020
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.

3 participants