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

Switch to Yarn Workspaces #3755

Merged
merged 18 commits into from
Jan 12, 2018
Merged

Switch to Yarn Workspaces #3755

merged 18 commits into from
Jan 12, 2018

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jan 11, 2018

This lets us remove a bunch of custom stuff.

It also seems to produce a smaller module tree (for development) and removes the hazard of overwriting Lerna symlinks when running yarn in individual package folders.

CONTRIBUTING.md Outdated
@@ -75,20 +75,18 @@ All functionality must be retained (and configuration given to the user) if they

1. Clone the repo with `git clone https://github.com/facebookincubator/create-react-app`

2. Run `npm install` in the root `create-react-app` folder.
2. Run `yarn --no-lockfile` in the root `create-react-app` folder.
Copy link
Contributor

@Timer Timer Jan 11, 2018

Choose a reason for hiding this comment

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

What if we made a .yarnrc with:

--install.no-lockfile true

This would keep things simple.

Copy link
Contributor

Choose a reason for hiding this comment

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

With this file present we can now probably edit this command to be just yarn.

CONTRIBUTING.md Outdated

If you want to try out the end-to-end flow with the global CLI, you can do this too:

```
npm run create-react-app my-app
yarn run create-react-app my-app
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: drop the run

CONTRIBUTING.md Outdated
@@ -119,11 +117,11 @@ By default git would use `CRLF` line endings which would cause the scripts to fa
4. Note that files in `packages/create-react-app` should be modified with extreme caution. Since it’s a global CLI, any version of `create-react-app` (global CLI) including very old ones should work with the latest version of `react-scripts`.
5. Create a change log entry for the release:
* You'll need an [access token for the GitHub API](https://help.github.com/articles/creating-an-access-token-for-command-line-use/). Save it to this environment variable: `export GITHUB_AUTH="..."`
* Run `npm run changelog`. The command will find all the labeled pull requests merged since the last release and group them by the label and affected packages, and create a change log entry with all the changes and links to PRs and their authors. Copy and paste it to `CHANGELOG.md`.
* Run `yarn run changelog`. The command will find all the labeled pull requests merged since the last release and group them by the label and affected packages, and create a change log entry with all the changes and links to PRs and their authors. Copy and paste it to `CHANGELOG.md`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: drop the run

package.json Outdated
"publish": "tasks/release.sh",
"start": "node packages/react-scripts/scripts/start.js",
"postinstall": "cd packages/react-error-overlay/ && yarn build:prod",
"publish": "lerna publish --independent",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd really like to see the git status --porcelain check back in here somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any ideas? I want to lose that file

Copy link
Contributor

Choose a reason for hiding this comment

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

(git status --porcelain | grep -q ^ && echo "Your git status is not clean. Aborting." && exit 1 || exit 0) && lerna publish --independent

@gaearon
Copy link
Contributor Author

gaearon commented Jan 11, 2018

Seems like this works but it's somewhat flaky on Node 6. I had a "no disk space" error once, and some inotify failure the other time. Not sure why this happens—maybe the tree is non-ideal somehow? I'd be surprised if npm's tree is more compact but who knows?

@Timer
Copy link
Contributor

Timer commented Jan 11, 2018

Try --link-duplicates?

@gaearon
Copy link
Contributor Author

gaearon commented Jan 11, 2018

Even if that flag works I doubt it’s well-maintained. Trying another approach.

@gaearon
Copy link
Contributor Author

gaearon commented Jan 12, 2018

Previous commit passed so this is good to go

@gaearon gaearon merged commit 0aeffe6 into master Jan 12, 2018
"scripts": {
"build": "node packages/react-scripts/scripts/build.js",
"build": "cd packages/react-scripts && node scripts/build.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this change in this PR? Do workspaces have an issue with working directories?

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 don't remember. I think it might not be related to YW but to Yarn in general (which sets cwd to the project you run the script in).

Pavek pushed a commit to Pavek/create-react-app that referenced this pull request Jul 10, 2018
* Switch to Yarn Workspaces

* Feedback

* Move flowconfig

* Use publish script

* Keep git status check

* Fix Flow without perf penalty

* Remove Flow from package.json "test"

* Try running it from script directly (?)

* Try magic incantations

* lol flow COME ON

* Try to skip Flow on AppVeyor

* -df

* -df

* -df

* Try to fix CI

* Revert unrelated changes

* Update CONTRIBUTING.md
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants