-
Notifications
You must be signed in to change notification settings - Fork 7
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
Setup turborepo, second try #52
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ dist/ | |
| node_modules/ | ||
|
|
||
| # misc | ||
| .turbo/ | ||
| /.env* | ||
| /.pnp* | ||
| /.pnpm-debug.log | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import syncPnpm from '../index.js'; | ||
|
|
||
| await syncPnpm(); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { join, dirname } from 'node:path'; | ||
| import { createRequire } from 'node:module'; | ||
|
|
||
| import { getPackages } from '@manypkg/get-packages'; | ||
| import { findRoot } from '@manypkg/find-root'; | ||
| import { readJson, pathExists } from 'fs-extra/esm'; | ||
| import { hardLinkDir } from '@pnpm/fs.hard-link-dir'; | ||
| import resolvePackagePath from 'resolve-package-path'; | ||
|
|
||
| const require = createRequire(import.meta.url); | ||
|
|
||
| const syncDir = './dist'; | ||
|
|
||
| export default async function syncPnpm(dir = process.cwd()) { | ||
| const root = await findRoot(dir); | ||
| const ownPackageJson = await readJson(join(dir, 'package.json')); | ||
| const ownDependencies = [ | ||
| ...Object.keys(ownPackageJson.dependencies ?? {}), | ||
| ...Object.keys(ownPackageJson.devDependencies ?? {}), | ||
| ]; | ||
|
|
||
| const localPackages = (await getPackages(root.rootDir)).packages; | ||
|
|
||
| const packagesToSync = localPackages.filter( | ||
| (p) => | ||
| p.packageJson.name !== 'sync-pnpm' && | ||
| ownDependencies.includes(p.packageJson.name) | ||
| ); | ||
|
|
||
| for (const pkg of packagesToSync) { | ||
| const syncFrom = join(pkg.dir, syncDir); | ||
| const resolvedPackagePath = dirname( | ||
| resolvePackagePath(pkg.packageJson.name, dir) | ||
| ); | ||
| const syncTo = join(resolvedPackagePath, syncDir); | ||
|
|
||
| if (await pathExists(syncFrom)) { | ||
| await hardLinkDir(syncFrom, [syncTo]); | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "name": "sync-pnpm", | ||
| "version": "0.0.0", | ||
| "private": true, | ||
| "description": "Sync build artifacts of workspace packages to pnpm injected/hard linked copies", | ||
| "repository": "", | ||
| "license": "MIT", | ||
| "author": "", | ||
| "main": "index.js", | ||
| "bin": "bin/sync-pnpm.js", | ||
| "type": "module", | ||
| "dependencies": { | ||
| "@manypkg/find-root": "^2.1.0", | ||
| "@manypkg/get-packages": "^2.1.0", | ||
| "@pnpm/fs.hard-link-dir": "^1.0.3", | ||
| "fs-extra": "^11.1.0", | ||
| "resolve-package-path": "^4.0.3" | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhhh - this is interesting!
So essentially this sync file forces
pnpmto hard link the dependencies one way or another so that we don't have to continuallypnpm i -fanytime we make a change in the addon source and want to see it in the test-app/docs-app?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. Also it does only this, it does not try to re-install/shuffle things around.
In an earlier attempt, I tried to just use
pnpm i -fin a turbo step, but when things run in parallel, this would lead to weird errors, aspnpm i -fwould temporarily remove things fromnode_modulesand add them back moments later, but in the meantime some parallel build would just fail as it would miss some dependencies innode_modules.So this does just adds back the missing hard links of our
/distfolder, and nothing else.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing!!
So is the current development process now (for something like
docs-app):ember-headless-formpnpm builddocs-app?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly, that should work (assuming you have
pnpm startorpnpm start:docsrunning, to continuously rebuild the app).It is a bit of a regression compared to other v2 addon setups (e.g. using
yarn), where thisinjectedthing does not exist (but instead other peer dependency related problems), so you could just runstartand it would watch the addon and the test-app. But not a regression compared to the previous state of this addon, because we had to runpnpm i -fagain and again, so this should make it better at least!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Definitely copying this for
toucan-core)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realized that the
pnpm buildscript does too much for this use case, as it builds the addons and the apps (test and docs). Will need another iteration!