Setup safer/more robust linking process for reaction/force #2758
Conversation
zephraph
added
the
Version: Trivial
|
This PR should be merged w/ artsy/force#4475 |
package.json
Outdated
| @@ -41,7 +41,8 @@ | |||
| "test:watch": "jest --watch --runInBand", | |||
| "test:debug": "node --inspect node_modules/.bin/jest --runInBand", | |||
| "type-check": "tsc --pretty --noEmit", | |||
| "watch": "concurrently --raw --kill-others 'yarn relay --watch' 'yarn compile -w' 'yarn emit-types --watch'" | |||
| "watch": "concurrently --raw --kill-others 'yarn relay --watch' 'yarn compile -w' 'yarn emit-types --watch' 'node scripts/watch-link.js'", | |||
| "dev-force": "yarn compile && yarn yalc publish && ./scripts/link-force.sh && yarn watch" | |||
There was a problem hiding this comment.
Just trying to make it representative of what you're trying to do, not how you're trying to do it.
There was a problem hiding this comment.
How about having watch just always publish (or is it really costly?) and then add a package script to force instead which runs yalc link @artsy/reaction? It’s 2 steps instead of 1, but I think it would keep things simpler to understand to the user.
There was a problem hiding this comment.
Watch does always publish. Ultimately I just want to make this whole process as seamless as possible. Ideally the dev doesn't even have to open force. The only reason I haven't grouped everything into a single command (including starting force) is because the terminal outputs would be mixed and I think that could cause further complications.
There was a problem hiding this comment.
I hear you, but I’m wondering if having to go into Reaction to link into Force is a helpful level of indirection. The same is going to apply to Volt, Positron, etc.
I think it makes more sense to have the app be responsible for linking and would also allow for the command to perform the linking be the same regardless of what app using Reaction.
If it turns out that people have trouble doing so, we can always add more abstractions later on.
scripts/watch-link.js
Outdated
| }) | ||
|
|
||
| const updateDevPackage = () => { | ||
| run("yarn", "yalc publish --force --changed") |
There was a problem hiding this comment.
If you were to pass these as an array of args then you should be able to get rid of the need for an intermediate shell. (Won’t make much of a difference here, so more just a general tip for when it matters.)
package.json
Outdated
| @@ -41,7 +41,8 @@ | |||
| "test:watch": "jest --watch --runInBand", | |||
| "test:debug": "node --inspect node_modules/.bin/jest --runInBand", | |||
| "type-check": "tsc --pretty --noEmit", | |||
| "watch": "concurrently --raw --kill-others 'yarn relay --watch' 'yarn compile -w' 'yarn emit-types --watch'" | |||
| "watch": "concurrently --raw --kill-others 'yarn relay --watch' 'yarn compile -w' 'yarn emit-types --watch' 'node scripts/watch-link.js'", | |||
zephraph
force-pushed
the
reduce-linking-headaches
branch
from
464ffab to
966bb4a
Compare
zephraph
changed the title
|
@zephraph - Will this change our existing workflows, which are quite vanilla? I think it would be good to ensure that this process is somewhat separate so that we're not wedded to additional pipelines on top of npm, yarn, etc. -- e.g., something like |
|
I'm working through it. Trying to make it as transparent as possible. |
zephraph
changed the title
Co-Authored-By: Eloy Durán <eloy.de.enige@gmail.com>
Co-Authored-By: Steven Hicks <steven.j.hicks@gmail.com>
Co-Authored-By: Steven Hicks <steven.j.hicks@gmail.com>
zephraph
force-pushed
the
reduce-linking-headaches
branch
from
21c3eab to
b7896e3
Compare
|
Okay, I feel like I've largely worked through it. None of these changes break any current workflows. You can continue to use the same commands as before and There's one command to "link" and start reaction + force up and the symlink linking issues won't be present. It's feeling pretty solid, but also doesn't require folks to change how they currently work. |
| @@ -0,0 +1,38 @@ | |||
| const chokidar = require("chokidar") | |||
| run("yarn", "yalc publish --force --changed --push") | ||
| } | ||
|
|
||
| console.log("Running yalc publish in watch mode") |
There was a problem hiding this comment.
Maybe "[Reaction] Running yalc publish in watch mode"
There was a problem hiding this comment.
You can continue to use the same commands as before and yarn link like normal without having to touch this workflow.
The new workflow is a def improvement! Just wanted to make sure that things remain flexible without too much (required) tooling overhead.
Looks great @zephraph 👍 👍
docs/developing_in_force.md
Outdated
|
|
||
| ``` | ||
| ENABLE_DEBUGGER=true yarn integrate force | ||
| ``` |
There was a problem hiding this comment.
(Would be good to cross reference this in Force's README as well.)
There was a problem hiding this comment.
In force the debugger is enabled by default if you're running it in vscode. I've just essentially short circuited that for this process assuming that normally you wouldn't want to run the debugger (given that it's so much slower).
|
Alright, let’s give this one a whirl! ✨ |
|
🚀 PR was released in v17.10.0 🚀 |
See related issue: artsy/force#4475
We've had pretty continual issues with linking reaction inside of force. @alloy did an excellent job of summarizing the issues with linking here.
I started researching solutions and stumbled upon this blog post by @mtfranchetto introducing yalc.
Conceptually yalc is a very simple tool. It publishes our package (reaction in this case) locally to the fs and the upstream project (force) uses it like it would a normal dependency. That means after adding it to force, you have to go through the process of installing dependencies like normal. This really replicates what happens when we install a package directly from NPM, but it's local!
To further reduce the friction that people face when linking, I created a new yarn script to simplify the reaction/force development workflow. Now to test force in reaction you don't even need to open force. Just run the following command from reaction:
There's a
quicklinks.shfile inside of force that handles what linking means to force. All theintegrateworkflow really does is callyarn watchand./scripts/quicklink.shfrom the project you tell it to integrate with. Technically we could add aquicklink.shscript to volt, positron, etc and it would essentially do the same thing.Note that the force side now has a bit more changes so that should definitely have some dedicated eyes on too.
This should go a long ways in simplifying our workflow and reducing friction we have when developing on reaction from inside of force.