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

Make Gel foundations ESM usable #1212

Merged
merged 13 commits into from
Jul 16, 2019
Merged

Make Gel foundations ESM usable #1212

merged 13 commits into from
Jul 16, 2019

Conversation

dr3
Copy link
Contributor

@dr3 dr3 commented Jul 15, 2019 •
edited
Loading

Contributes to #557

Overall change: Adds ESM support to gel-foundations

Code changes:

  • Uses ESM modules package key
  • Restructures package to use nested package.jsons to expose ESM code
  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@dr3 dr3 self-assigned this Jul 15, 2019
@dr3 dr3 changed the title 🚀 Make Gel foundations ESM usable Make Gel foundations ESM usable Jul 15, 2019
@dr3
Copy link
Contributor Author

dr3 commented Jul 15, 2019

Tested locally by publishing gel-foundations as drew-testing-123@1.0.9 and importing into simorgh, string replacing @bbc/gel-foundations with drew-testing-123, Working as expected

@dr3 dr3 changed the base branch from BuildESModules to latest July 15, 2019 14:45
@dr3 dr3 marked this pull request as ready for review July 15, 2019 14:46
pjlee11
pjlee11 approved these changes Jul 15, 2019
View reviewed changes
Copy link
Contributor

@pjlee11 pjlee11 left a comment

Choose a reason for hiding this comment

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

@pjlee11
Copy link
Contributor

pjlee11 commented Jul 15, 2019

Comparing latest to this branch npm link'ed into simorgh and seeing the top level gel-foundations smaller in the webpack bundle reports 🎉

Webpack report using ESM (linking this branch to simorgh)

image

Webpack report not using ESM (latest)

image

@dr3
Copy link
Contributor Author

dr3 commented Jul 16, 2019

Doesnt need testing, will regression once published

@dr3 dr3 merged commit eaa30af into latest Jul 16, 2019
@dr3 dr3 deleted the ESM-gel-foundations branch July 16, 2019 21:34
@bbc-news-frameworks bbc-news-frameworks mentioned this pull request Sep 11, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pjlee11 pjlee11 pjlee11 approved these changes

@chris-hinds chris-hinds chris-hinds approved these changes

@j-pendlebury j-pendlebury Awaiting requested review from j-pendlebury

@sareh sareh Awaiting requested review from sareh

Assignees

@dr3 dr3

Labels
simorgh-core-stream
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dr3 @pjlee11 @chris-hinds