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

Get rid of @remove-on-publish step #765

Closed
gaearon opened this issue Sep 26, 2016 · 0 comments · Fixed by #867
Closed

Get rid of @remove-on-publish step #765

gaearon opened this issue Sep 26, 2016 · 0 comments · Fixed by #867

Comments

@gaearon
Copy link
Contributor

gaearon commented Sep 26, 2016

In #257, we introduced two special “annotations” that our scripts use to cut out some parts of the code at different stages: @remove-on-eject and @remove-on-publish.

@remove-on-eject

Code between @remove-on-eject-begin and @remove-on-eject-end gets cut out during ejecting. This is useful for simplifying configuration during ejection because some things become unnecessary. We also use it to strip out Facebook license headers because we don’t want them in generated code after ejecting. Here’s a few examples of how we use it: stripping comments, changing paths, simplifying configuration after ejecting.

@remove-on-publish

Code between @remove-on-publish-begin and @remove-on-publish-end gets cut out during publish of react-scripts package. We use this for cutting out code we only use for local development of Create React App itself. It turns out that while @remove-on-eject is super useful, @remove-on-publish is not. We only use @remove-on-publish in one place to override the paths to the template in development, and I just don’t think it’s worth it.

Let’s get rid of @remove-on-publish

In the past we used to check our parent directory to figure out if we’re inside Create React App repo or inside somebody’s node_modules. I think we should get back to that approach. As long as that code is behind @remove-on-eject, user won’t see it anyway.

When we get rid of @remove-on-publish, we can make our local end-to-end flow task and release task simpler because we can remove the code that moves files into a separate “clean” folder and run both scripts right from the local project folder instead.

What needs to be done?

  • Remove all @remove-on-publish annotations (AFAIK there’s only one).
  • Figure out another way to make npm start / npm run build / npm test work in the root of Create React App repo. Switching to “local development“ mode when react-scripts is inside a folder named packages seems reasonable to me.
  • Change tasks/cra.sh to run from the project folder and remove the copying step. The tasks/cra.sh script is the “local end-to-end flow” you can check by running npm run create-react-app ../whatever from the repo root. It is meant for testing that CRA works locally.
  • Change tasks/release.sh to run from the project folder and remove the copying step. The tasks/release.sh script is what we use to publish react-scripts and friends to npm.

I know this is pretty involved so feel free to ask any details in this thread. The whole setup is a bit messy and has some issues so don’t be surprised if something doesn’t quite work. 😄 If you get stuck please leave a comment and push work in progress as a PR so we can take a look at what’s failing.

gaearon pushed a commit that referenced this issue Oct 7, 2016
feiqitian pushed a commit to feiqitian/create-react-app that referenced this issue Oct 25, 2016
jarlef pushed a commit to jarlef/create-react-app that referenced this issue Nov 28, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
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 a pull request may close this issue.

1 participant