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

Add babel-plugin-transform-react-remove-prop-types #14814

Closed
neo opened this issue Jun 16, 2019 · 11 comments · Fixed by #14987
Closed

Add babel-plugin-transform-react-remove-prop-types #14814

neo opened this issue Jun 16, 2019 · 11 comments · Fixed by #14987
Labels
good first issue Issue that doesn't require previous experience with Gatsby

Comments

@neo
Copy link
Contributor

neo commented Jun 16, 2019

Summary

I am not 100% sure, but it seems like babel-plugin-transform-react-remove-prop-types isn't being used when gatsby is building for production.

In my opinion, this is a very useful and practical thing to add.

Basic example

N/A

Motivation

To achieve smaller production build bundle.
@sidharthachatterjee
Copy link
Contributor

Sounds useful but I'm not entirely sure if we should add it in our default config (which is super lightweight by design).

Let's keep this open and hear what everyone else has to say.

In the mean time, you could add this in a custom .babelrc file

@sidharthachatterjee sidharthachatterjee added type: feature or enhancement good first issue Issue that doesn't require previous experience with Gatsby labels Jun 17, 2019
@sidharthachatterjee
Copy link
Contributor

Just spoke with @DSchau about this and we both agree that this is useful indeed!

We would love to accept a PR with this. 💪

@luisFilipePT
Copy link
Contributor

luisFilipePT commented Jun 18, 2019 •
edited

Hi,

Can I try to tackle this one?

Cheers

@sidharthachatterjee
Copy link
Contributor

@luisFilipePT Sure! Thank you for picking it up 🙌

@wardpeet
Copy link
Contributor

@luisFilipePT You should be able to add it to babel-preset-gatsby/src/index.js

@luisFilipePT
Copy link
Contributor

luisFilipePT commented Jun 19, 2019 •
edited

@wardpeet thanks for the tip 👍
[UPDATED]
I started to check it out last night, just not sure yet how to test it, this is what I have tried (so far)

1 - Added the npm package in packages/babel-preset-gatsby
2 - Added the config to src/index.js
3 - Built everything
4 - Test with build the default starter
5 - Go to public and search for propTypes in that folder
6 - Compare with the original build (before the change)

Am I on the right path? 😄 No luck yet :/

PS - The goal here, for now, is just to remove propTypes on Gatsby prod but not on the plugins, right?

Thanks guys

@luisFilipePT
Copy link
Contributor

luisFilipePT commented Jun 20, 2019 •
edited

kind of struggling here, this are my current changes:

I'm missing something 😕

image

any clues? 😄

@luisFilipePT luisFilipePT mentioned this issue Jun 20, 2019
Merged
4 tasks
luisFilipePT added a commit that referenced this issue Jun 28, 2019
@luisFilipePT
test(babel-preset-gatsby): Remove beforeEach in tests on production env
488b9a0 
#14814
@gatsbot
Copy link

gatsbot bot commented Jul 11, 2019

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contributefor more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Jul 11, 2019
@marcel0ll
Copy link

marcel0ll commented Jul 12, 2019 •
edited by luisFilipePT

@luisFilipePT how is this going? I might be able to help you out

@luisFilipePT
Copy link
Contributor

@luisFilipePT how is this going? I might be able to help you out

I think it's done. You can check the PR and see if theres anything that could be improved.
But for now I am just waiting for feedback or for the PR to be merged 😄

@pieh
Copy link
Contributor

pieh commented Jul 12, 2019

Plan is to merge this today (we are doing bunch of babel preset related changes)

@neo neo added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Jul 12, 2019
gatsbybot pushed a commit that referenced this issue Jul 12, 2019
@luisFilipePT
feat(babel-preset-gatsby): add babel-plugin-transform-react-remove-pr…
633e9ea 
…op-types for production builds (#14987)

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

Remove PropTypes from production build

re #14814

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

re #14814

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

#14814

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

#14814

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

#14814

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

#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
Assignees
No one assigned
Labels
good first issue Issue that doesn't require previous experience with Gatsby
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(babel-preset-gatsby): add babel-plugin-transform-react-remove-prop-types for production builds
7 participants
@pieh @wardpeet @marcel0ll @sidharthachatterjee @neo @luisFilipePT @LekoArts