Skip to content
This repository has been archived by the owner on May 2, 2023. It is now read-only.

fix(storybook): add babel-polyfill to preview-head for IE Edge - TWIG-220 #289

Merged
merged 5 commits into from
Jan 22, 2020

Conversation

papegaill
Copy link
Contributor

PR description

Please drop a few lines about the PR: what it does, how to test it, etc.

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

  • I have put the vanilla component as devDependencies
  • I have put the specs package as devDependencies
  • I have added the components directly used in the twig file (with include or embed) as dependencies
  • My component is listed in @ecl-twig/ec-components's dependencies
  • My variables naming follow the guidelines (snake case for twig)
  • I have provided tests
  • I have provided documentation (for the "notes" tab)
  • If my local yarn.lock contains changes, I have committed it
  • I have given my PR the proper label (pr: review needed to indicate that I'm done and now waiting for a review ,pr: wip to indicate that I'm actively working on it ...)

@papegaill papegaill added the pr: review needed Use this label to show that your PR needs to be review label Jan 16, 2020
@kalinchernev kalinchernev self-assigned this Jan 20, 2020
Copy link
Contributor

@kalinchernev kalinchernev left a comment

Choose a reason for hiding this comment

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

Checked and it works fine.
Please note that this package has been deprecated (as noted in babel docs) and it has a newer version which also works fine.

This preview template is not something to deliver in the final product of the repo (like the twig templates) so it looks ok to me to have the script added in the head of the preview.

Just a note, I gave polyfill.io a try with a selected es5, es6, es7 + Symbol features. Didn't work as the babel/polyfill so I don't have a better idea or alternative to the used lib.

@kalinchernev kalinchernev removed their assignment Jan 20, 2020
@planctus planctus added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Jan 20, 2020
@papegaill papegaill added the pr: wip Mark the PR as WIP label Jan 22, 2020
@papegaill
Copy link
Contributor Author

@kalinchernev, @planctus PR updated, I tried to not use @babel/polyfill as mention in this article, but without success https://www.thebasement.be/updating-to-babel-7.4/

@papegaill papegaill added pr: review needed Use this label to show that your PR needs to be review and removed pr: wip Mark the PR as WIP labels Jan 22, 2020
@kalinchernev kalinchernev merged commit 97c985e into develop Jan 22, 2020
@kalinchernev kalinchernev deleted the TWIG-220 branch January 22, 2020 08:50
@kalinchernev kalinchernev removed pr: questions pr: review needed Use this label to show that your PR needs to be review labels Jan 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants