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

Easier linking between force/reaction #4475

Merged
merged 3 commits into from
Sep 4, 2019
Merged

Conversation

zephraph
Copy link
Contributor

@zephraph zephraph commented Aug 30, 2019

For more context on this PR, see artsy/reaction#2758. These two PRs should be merged together.

Essentially this adds a mechanism that when in reaction the user can run yarn integrate force to automatically link and start both force and reaction. At the time of this PR, there's no support for starting the linking process while in force.

If you need to do work in force, you'll have to go to react first to run the command. Adding a command in force will be a follow up.

The linking process leaves the package.json/yarn.lock file of force in a dirty state because it actually updates the reference of the linked package (like reaction) to point to a local reference. Running yarn unlink-all will clean up those references. There's a pre-commit hook that will fail if the user tries to commit package.json or yarn.lock in a bad state and give the command to clean it up.

This PR adds a few things:

  1. A script called quicklink which controls the linking behavior of a package being linked to force. It is designed to cover reaction but likely will seamlessly handle palette as well.
  2. A lint-staged hook on package.json and yarn.lock that fails a commit if linking is still active. The process of linking actually makes a change to the package.json and yarn.lock to point the package to the published package directory.
  3. A yarn unlink-all command to easily clean up linking

Through the whole process the tool (yalc) that handles this new linking is kind of abstracted away from the user. I don't yet have a yarn command to directly link a package like reaction from inside force. If that becomes a need we can add it later.

@codecov
Copy link

codecov bot commented Aug 30, 2019

Codecov Report

Merging #4475 into master will increase coverage by <.1%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master   #4475     +/-   ##
========================================
+ Coverage    79.3%   79.3%   +<.1%     
========================================
  Files        1269    1269             
  Lines       35404   35493     +89     
  Branches     2059    2063      +4     
========================================
+ Hits        28097   28178     +81     
- Misses       6262    6277     +15     
+ Partials     1045    1038      -7

@zephraph zephraph changed the title Add yalc for easier linking between force/reaction WIP: Add yalc for easier linking between force/reaction Sep 3, 2019
@zephraph zephraph changed the title WIP: Add yalc for easier linking between force/reaction Add yalc for easier linking between force/reaction Sep 3, 2019
@zephraph zephraph changed the title Add yalc for easier linking between force/reaction Easier linking between force/reaction Sep 4, 2019
@zephraph zephraph force-pushed the reduce-linking-headaches branch from 6f90584 to 00c6968 Compare September 4, 2019 14:20
@alloy alloy merged commit a6356fd into master Sep 4, 2019
@alloy alloy deleted the reduce-linking-headaches branch September 4, 2019 15:27
@artsy-peril artsy-peril bot mentioned this pull request Sep 4, 2019
@damassi
Copy link
Member

damassi commented Sep 4, 2019

@zephraph - can you update the main description with the full flow (for the everyday user) of how this works with reaction? I'm not following exactly, assuming i'm working from force. Also not following what needs to be done re linking / unlinking, or why, etc.

@artsy artsy deleted a comment from artsy-peril bot Sep 5, 2019
@artsy artsy deleted a comment from artsy-peril bot Sep 5, 2019
@zephraph
Copy link
Contributor Author

zephraph commented Sep 5, 2019

@damassi updated. Does that answer your questions?

@damassi
Copy link
Member

damassi commented Sep 5, 2019

Yup, thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants