Use yarn workspaces #268
Use yarn workspaces #268
Conversation
|
Thank you! 🍕 I was just about to start working on it myself! I am looking forward to merge it ASAP, just need someone else to have a look at it. |
.gitignore
Outdated
| npm-debug.log | ||
| lerna-*.log | ||
| npm-*.log | ||
| yarn-*.log |
There was a problem hiding this comment.
Could you please change these 3 to simply *.log?
There was a problem hiding this comment.
And also would be nice to add package-lock.json.
There was a problem hiding this comment.
Could you please change these 3 to simply
*.log?
Sure.
And also would be nice to add
package-lock.json.
Could you please explain why?
There was a problem hiding this comment.
To make sure it won't be accidentally added in the future when someone installs deps with npm.
dmitmel
force-pushed
the
feature/yarn-workspaces
branch
from
d89bff3 to
07400b5
Compare
dmitmel
force-pushed
the
feature/yarn-workspaces
branch
2 times, most recently
from
254dd29 to
523ab44
Compare
There was a problem hiding this comment.
Great! Would like someone else to have a look: @jvanbruegge @zcei @andywer
dmitmel
force-pushed
the
feature/yarn-workspaces
branch
from
523ab44 to
acd8d80
Compare
dmitmel
force-pushed
the
feature/yarn-workspaces
branch
from
acd8d80 to
1591443
Compare
| "scripts": { | ||
| "test": "lerna run test", | ||
| "postinstall": "lerna bootstrap && link-parent-bin" |
There was a problem hiding this comment.
Why not hook up on the postinstall hook?
Btw, we might not need lerna bootstrap anymore if we run yarn and have workspaces. We still need link-parent-bin, though, since yarn also tends to fail to link bins installed in top-level node_modules.
There was a problem hiding this comment.
Are you sure yarn fails to link bins? Is there a way to test it?
There was a problem hiding this comment.
I set-up a few monorepos with yarn workspaces in the last couple of month and encountered that issue at least twice.
Not sure if there is a real way to test. But if a package has a dependency with a CLI tool (ava for instance) and the dependency is installed in the monorepo-wide node_modules, then I think the tool will always be missing from the packages node_modules/.bin` as of now.
There was a problem hiding this comment.
I've just installed this branch and yarn didn't create links for packages that are not in the root, so I guess this is the default behaviour. Why do you need it for by the way?
There was a problem hiding this comment.
Let's give it a try :)
Sorry, @andywer, what do you mean?
There was a problem hiding this comment.
I meant
So, should I remove the bootstrap script from the root package?
👉I think it should still work without lerna bootstrap when using yarn and yarn workspaces. Let's try and see if that's so 😉
There was a problem hiding this comment.
@andywer Well, it works on my machine, tests are passing and build runs without any errors.
There was a problem hiding this comment.
Good enough for me 😅
But seriously, I think it does pretty much exactly the same, so let's get rid of one of them.
There was a problem hiding this comment.
lerna bootstrap actually just uses yarn install when using workspaces, so +1 to killing any references to it.
| @@ -1,4 +1 @@ | |||
| declare var module : any | |||
| declare var process : any | |||
|
|
|||
| module.exports = process.env.TEST | |||
There was a problem hiding this comment.
I'd like to keep having something TypeScript specific in the app.ts - currently we set the webpack config to read from app.ts and have the ts block set up, but I think it would also work without, as it currently is valid JS.
Probably a simple
module.exports = process.env.TEST as stringwould be enough
There was a problem hiding this comment.
@zcei What about this?
function add(a: number, b: number): number {
return a + b;
}
|
Updated and merged. Thanks everyone for working on it! |
No description provided.