Skip to content

No core-js in core#2211

Merged
epicfaace merged 1 commit intorjsf-team:masterfrom
wegry:1601-no-core-polyfill
Mar 23, 2021
Merged

No core-js in core#2211
epicfaace merged 1 commit intorjsf-team:masterfrom
wegry:1601-no-core-polyfill

Conversation

@wegry
Copy link
Contributor

@wegry wegry commented Jan 29, 2021

Reasons for making this change

Two versions of core-js currently exist in my app because @rjsf/core pulls in @babel/runtime currently. There are two direct imports of core-js that I've opted to pull in pure imports for.

For the rationale of why library code shouldn't include polyfills, this comment
w3ctag/polyfills#6 (comment) explains my rationale. This is a breaking change though, if downstream folks were depending on @babel/runtime being around.

If this is related to existing tickets, include links to them as well. fixes #1601

Recreates PR for #2049

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@wegry wegry requested a review from agustin107 as a code owner January 29, 2021 03:05
@wegry wegry force-pushed the 1601-no-core-polyfill branch from d210385 to 1a8c4aa Compare January 29, 2021 03:14
@wegry wegry changed the title No core polyfills (recreate #2049) No core-js in core (recreate #2049) Jan 29, 2021
Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

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

Thanks for this change! Do you mind also adding a documentation page called "3.x upgrade guide" that describes the nature of this change, and also what people should do (what polyfills they should install) if they were depending on @babel/runtime-corejs2 that used to be bundled with @rjsf/core?

wegry added a commit to wegry/react-jsonschema-form that referenced this pull request Feb 20, 2021
@wegry wegry force-pushed the 1601-no-core-polyfill branch from 3c6c0ea to 26d3405 Compare February 20, 2021 19:23

If you're using a framework like [Create React App](https://create-react-app.dev/docs/supported-browsers-features#supported-browsers), [Gatsby](https://www.gatsbyjs.com/docs/how-to/custom-configuration/browser-support/), or [Next.js](https://nextjs.org/docs/basic-features/supported-browsers-features), polyfills are already included there.

If you were directly depending on @rjsf/core's @babel/runtime pulling in core-js@2, [@babel/preset-env](https://babeljs.io/docs/en/babel-preset-env#how-does-it-work) is probably a good first choice.
Copy link
Member

Choose a reason for hiding this comment

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

Just want to clarify here -- is the only solution here that we have to begin transpiling the code with @babel/preset-env? What if we just ran npm install core-js and added import 'core-js'; -- would that also be a solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a note to that effect.

@epicfaace epicfaace mentioned this pull request Mar 22, 2021
7 tasks
@epicfaace epicfaace changed the title No core-js in core (recreate #2049) No core-js in core Mar 22, 2021
Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@epicfaace
Copy link
Member

@wegry Can you merge master into this branch? Then we should be all ready to merge.

@wegry
Copy link
Contributor Author

wegry commented Mar 23, 2021

Let me give it a shot @epicfaace.

@wegry wegry force-pushed the 1601-no-core-polyfill branch from 4f0dc68 to 2283e99 Compare March 23, 2021 23:01
@epicfaace epicfaace merged commit 1ae4f39 into rjsf-team:master Mar 23, 2021
@epicfaace epicfaace mentioned this pull request Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upgrade core-js to 3.x

2 participants