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

Jump to bottom

feat(babel-preset-gatsby): add babel-plugin-transform-react-remove-prop-types for production builds #14987

Merged
merged 12 commits into from Jul 12, 2019
Merged

feat(babel-preset-gatsby): add babel-plugin-transform-react-remove-prop-types for production builds #14987

merged 12 commits into from Jul 12, 2019

Conversation

luisFilipePT
Copy link
Contributor

@luisFilipePT luisFilipePT commented Jun 20, 2019 •
edited by pieh

PR Checklist

  • Tests for the changes have been added.

Purpose of this PR

What kind of change does this Pull Request introduce?

  • feat: A new feature

Changes

Remove PropTypes from production build by adding babel-plugin-transform-react-remove-prop-types.

This change removes PropTypes and their imports from the production build.
This change DOES NOT remove proptypes from dependencies from external packages (in node_modules)

Is there anything you would like reviewers to focus on?

No

Does this PR introduce a breaking change?

  • Yes
  • No

Related issue

Closes #14814.

Reviewers

Gatsby team?

Additional Reviewers

Please review: @sidharthachatterjee @neo @wardpeet

@luisFilipePT
feat(babel-preset-gatsby): Add babel-plugin-transform-react-remove-pr…
83c5645 
…op-types to babel-preset-gatsby

Remove PropTypes from production build

re #14814
@luisFilipePT
docs(babel-preset-gatsby): Add babel-plugin-transform-react-remove-pr…
c5275d3 
…op-types link to README in #pac

re #14814
@luisFilipePT luisFilipePT requested a review from a team as a code owner June 20, 2019 19:17
@pieh pieh changed the title [babel-preset-gatsby] - Add babel-plugin-transform-react-remove-prop-types feat(babel-preset-gatsby): add babel-plugin-transform-react-remove-prop-types for production builds Jun 26, 2019
@luisFilipePT luisFilipePT self-assigned this Jun 26, 2019
@pieh pieh requested a review from a team as a code owner July 12, 2019 10:53
sidharthachatterjee
sidharthachatterjee suggested changes Jul 12, 2019
View reviewed changes
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Thank you for this @luisFilipePT 🥇

Left a small comment!

packages/babel-preset-gatsby/src/index.js Outdated Show resolved Hide resolved
@sidharthachatterjee sidharthachatterjee added the status: awaiting author response Additional information has been requested from the author label Jul 12, 2019
@sidharthachatterjee sidharthachatterjee added bot: merge on green Gatsbot will merge these PRs automatically when all tests passes and removed status: awaiting author response Additional information has been requested from the author labels Jul 12, 2019
@neo
Copy link
Contributor

neo commented Jul 12, 2019

should we expose some config like additionalLibraries or those people with needs of configurations should just create their own .babelrc?

@sidharthachatterjee
Copy link
Contributor

should we expose some config like additionalLibraries or those people with needs of configurations should just create their own .babelrc?

@neo We could definitely add additionalLibraries as an option in a follow up PR

@gatsbybot gatsbybot merged commit 633e9ea into gatsbyjs:master Jul 12, 2019
@pieh
Copy link
Contributor

pieh commented Jul 12, 2019

@neo You can modify babel with .babelrc - see https://www.gatsbyjs.org/docs/babel/#how-to-use-a-custom-babelrc-file

@neo
Copy link
Contributor

neo commented Jul 12, 2019

yea, that's what I figured – I was just curious about where do we draw the line.
I think it makes sense to not exposing any options, otherwise we would need to choose what to expose...
so all good with me! 😆 glad it's merged.

@wardpeet
Copy link
Contributor

Proptypes is used in a lot of dependencies as it's the only runtime check supported by react. We want to be as fast and as small as possible. Having good defaults is key so removing prop-types by default in production makes a lot of sense.

johno pushed a commit to johno/gatsby that referenced this pull request Jul 17, 2019
@luisFilipePT @johno
feat(babel-preset-gatsby): add babel-plugin-transform-react-remove-pr…
f6feae5 
…op-types for production builds (gatsbyjs#14987)

* feat(babel-preset-gatsby): Add babel-plugin-transform-react-remove-prop-types to babel-preset-gatsby

Remove PropTypes from production build

re gatsbyjs#14814

* docs(babel-preset-gatsby): Add babel-plugin-transform-react-remove-prop-types link to README in #pac

re gatsbyjs#14814

* test(babel-preset-gatsby): Add test - in production mode specifies proper presets

gatsbyjs#14814

* fix(babel-preset-gatsby): Remove core-js package from package.json installed when developing but nev

gatsbyjs#14814

* refactor(babel-preset-gatsby): Use GATSBY_BUILD_STAGE in place of BABEL_ENV || NODE_ENV

gatsbyjs#14814

* test(babel-preset-gatsby): Remove beforeEach in tests on production env

gatsbyjs#14814

* update lock file

* update tests

* fixup after merge conflict resolution/small refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pieh pieh pieh left review comments

@wardpeet wardpeet wardpeet left review comments

@sidharthachatterjee sidharthachatterjee sidharthachatterjee approved these changes

Assignees

@luisFilipePT luisFilipePT

Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add babel-plugin-transform-react-remove-prop-types
6 participants
@luisFilipePT @neo @sidharthachatterjee @pieh @wardpeet @gatsbybot