Setup turborepo, second try #52

simonihmig · 2023-02-16T13:26:24Z

Another attempt, using a custom sync step to work around vercel/turbo#2306

builds on top of Setup turborepo #45
adds a sync-pnpm tool to sync the /dist output
adds an intermediate turbo task _build to call sync after build

Best reviewed by commit (first one is #45 squashed into one)

changeset-bot · 2023-02-16T13:26:27Z

⚠️ No Changeset found

Latest commit: 4c81454

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

simonihmig · 2023-02-16T13:38:36Z

packages/changeset-validations/package.json

@@ -101,7 +101,8 @@
      "types": "./dist/*.d.ts",
      "default": "./dist/*.js"
    },
-    "./addon-main.js": "./addon-main.cjs"
+    "./addon-main.js": "./addon-main.cjs",
+    "./package.json": "./package.json"


This is a workaround for an unfortunate issue...

sync-pnpm needs to find the hard linked package resolvable from the test-app, which is like a copy of our original addon in the monorepo, just without /dist (which is the issue we need to fix here). But require.resolve('ember-headless-form') does not work, because its main field points to its ./dist/index.js, which des not exist before we have actually synced. So resolve throws.

Then I tried with this package that I found on npm, which basically tries to solve that issue: resolve-pkg. But running into sindresorhus/resolve-pkg#9.

So the workaround here is to make mypackage/package.json resolvable. Not ideal, but works for now. Not sure if we can do this in any other way. We want to delegate the node resolution algorithm to node itself and not re-invent this, but there seems to be no way to say "give me the module path that you would try to require, without actually throwing when it is not available".

does this one support exports? https://www.npmjs.com/package/resolve-package-path -- it's what auto-import uses

It seems to replicate the node module resolution, instead of delegating to node (like require.resolve), so probably that would work! Thanks for the suggestion, will try this out!

Related: sindresorhus/resolve-pkg#9 (comment)

will try this out!

It works! 🎉

NullVoxPopuli · 2023-02-16T13:51:00Z

Total runtime is 22m: https://github.com/CrowdStrike/ember-headless-form/actions/runs/4194427919/usage

can you push an empty commit? hopefully that'd be a full cache hit

simonihmig · 2023-02-16T18:26:30Z

Total runtime is 22m:

For the record, caching seems to be fixed now. Total runtime < 9min - https://github.com/CrowdStrike/ember-headless-form/actions/runs/4196887915/usage

NullVoxPopuli

Nice work!

Would be cool to see on ember-headless-table and ember-toucan-core, as well 🎉

simonihmig · 2023-02-16T18:34:50Z

Would be cool to see on ember-headless-table and ember-toucan-core, as well 🎉

Yeah, which brings us to the topic of what to do with sync-pnpm. I put it here as a PoC. But if that's the thing we use to fix the issue, then we need to extract that to somewhere!?

NullVoxPopuli · 2023-02-16T18:47:28Z

then we need to extract that to somewhere!?

I wonder if the turbo-repo folks would be interested in adopting it. I'll ask here: vercel/turbo#2306

simonihmig · 2023-02-17T10:34:14Z

I'll hold this back until we have #44 merged, so I can integrate the docs setup here too...

github-actions · 2023-02-20T10:19:08Z

Preview URLs

Env: preview
Docs: https://fa46d0ba.ember-headless-form.pages.dev

ynotdraw · 2023-02-21T13:43:15Z

internal/sync-pnpm/index.js

+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';


Ahhhh - this is interesting!

Hard link (or copy if linking fails) all files from a directory to several target directories.

So essentially this sync file forces pnpm to hard link the dependencies one way or another so that we don't have to continually pnpm i -f anytime we make a change in the addon source and want to see it in the test-app/docs-app?

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 -f in a turbo step, but when things run in parallel, this would lead to weird errors, as pnpm i -f would temporarily remove things from node_modules and add them back moments later, but in the meantime some parallel build would just fail as it would miss some dependencies in node_modules.

So this does just adds back the missing hard links of our /dist folder, and nothing else.

Amazing!!

So is the current development process now (for something like docs-app):

make a change in ember-headless-form

pnpm build

see the update in the docs-app?

Yes, exactly, that should work (assuming you have pnpm start or pnpm start:docs running, to continuously rebuild the app).

It is a bit of a regression compared to other v2 addon setups (e.g. using yarn), where this injected thing does not exist (but instead other peer dependency related problems), so you could just run start and 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 run pnpm i -f again and again, so this should make it better at least!

(Definitely copying this for toucan-core)

Just realized that the pnpm build script does too much for this use case, as it builds the addons and the apps (test and docs). Will need another iteration!

simonihmig commented Feb 16, 2023

View reviewed changes

simonihmig force-pushed the turbo_try2 branch from ea5282e to 0188aca Compare February 16, 2023 15:48

NullVoxPopuli approved these changes Feb 16, 2023

View reviewed changes

This was referenced Feb 16, 2023

pnpm's dependenciesMeta.*.injected can't re-sync vercel/turbo#2306

Closed

support turborepo when using dependenciesMeta.*. injected (make injected deps re-sync) pnpm/pnpm#6088

Open

simonihmig mentioned this pull request Feb 17, 2023

Setup docs app #44

Merged

simonihmig added 3 commits February 20, 2023 10:28

Setup turbo based on Preston's work

4bbaba7

Create sync-pnpm tool

f35f6f8

Add turbo to docs app

733d8a8

simonihmig force-pushed the turbo_try2 branch from 8a8b9c5 to 733d8a8 Compare February 20, 2023 10:16

Integrate start command with turbo

b339b0c

simonihmig force-pushed the turbo_try2 branch 2 times, most recently from 56fef0e to 1e61c65 Compare February 20, 2023 11:17

Streamline turbo setup

4c81454

simonihmig force-pushed the turbo_try2 branch from 87540ac to 4c81454 Compare February 20, 2023 11:50

simonihmig marked this pull request as ready for review February 20, 2023 11:56

simonihmig requested review from nicolechung and ynotdraw February 20, 2023 11:56

simonihmig mentioned this pull request Feb 20, 2023

Setup turborepo #45

Closed

ynotdraw approved these changes Feb 21, 2023

View reviewed changes

nicolechung approved these changes Feb 21, 2023

View reviewed changes

simonihmig merged commit b0b2a84 into main Feb 21, 2023

simonihmig deleted the turbo_try2 branch February 21, 2023 15:28

ynotdraw mentioned this pull request Feb 21, 2023

Add turbo + pnpm syncing CrowdStrike/ember-toucan-core#40

Closed

simonihmig mentioned this pull request Feb 24, 2023

Figure out CI setup for sharing a build when having multiple packages #33

Closed

NullVoxPopuli mentioned this pull request Feb 25, 2023

Setup turborepo, with injected dependency syncing CrowdStrike/ember-headless-table#145

Draft

ynotdraw mentioned this pull request Apr 4, 2023

#40: Add turbo and pnpm syncing support CrowdStrike/ember-toucan-core#119

Merged

joelamb mentioned this pull request May 12, 2023

Turbo and sync pnpm CrowdStrike/ember-headless-table#168

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup turborepo, second try #52

Setup turborepo, second try #52

simonihmig commented Feb 16, 2023 •

edited

changeset-bot bot commented Feb 16, 2023 •

edited

simonihmig Feb 16, 2023

NullVoxPopuli Feb 16, 2023

simonihmig Feb 16, 2023

simonihmig Feb 16, 2023

NullVoxPopuli commented Feb 16, 2023

simonihmig commented Feb 16, 2023

NullVoxPopuli left a comment

simonihmig commented Feb 16, 2023

NullVoxPopuli commented Feb 16, 2023

simonihmig commented Feb 17, 2023

github-actions bot commented Feb 20, 2023 •

edited

ynotdraw Feb 21, 2023

simonihmig Feb 21, 2023

nicolechung Feb 21, 2023

simonihmig Feb 21, 2023

ynotdraw Feb 21, 2023

simonihmig Feb 21, 2023

Setup turborepo, second try #52

Setup turborepo, second try #52

Conversation

simonihmig commented Feb 16, 2023 • edited

changeset-bot bot commented Feb 16, 2023 • edited

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli commented Feb 16, 2023

simonihmig commented Feb 16, 2023

NullVoxPopuli left a comment

Choose a reason for hiding this comment

simonihmig commented Feb 16, 2023

NullVoxPopuli commented Feb 16, 2023

simonihmig commented Feb 17, 2023

github-actions bot commented Feb 20, 2023 • edited

Preview URLs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonihmig commented Feb 16, 2023 •

edited

changeset-bot bot commented Feb 16, 2023 •

edited

github-actions bot commented Feb 20, 2023 •

edited