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

feat(@turbo/workspaces): add new workspaces package (#3818 #3818

Merged
merged 4 commits into from
Feb 22, 2023

Conversation

tknickman
Copy link
Member

@tknickman tknickman commented Feb 15, 2023

Codemods to convert between workspace managers. Node API will be used by create-turbo when cloning examples.

This is also provided as a standalone CLI.

Usage:
  $ npx @turbo/workspace-convert [flags...] [<dir>]
  
  If <dir> is not provided, you will be prompted for it.
  
  Flags:
    --npm           Switch to npm workspaces
    --pnpm          Switch to pnpm workspaces
    --yarn          Switch to yarn workspaces
    --install       Convert lock files and install dependencies
    --dry           Run without making any modifications
    --summary       Show a summary of the workspace at <dir>, including the detected package
                    manager, all workspaces, and their locations
    --help, -h      Show this help message
    --version, -v   Show the version of this script

@vercel
Copy link

vercel bot commented Feb 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
examples-basic-web 🔄 Building (Inspect) Feb 22, 2023 at 7:07PM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 22, 2023 at 7:07PM (UTC)
8 Ignored Deployments
Name Status Preview Comments Updated
examples-cra-web ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)
examples-vite-web ⬜️ Ignored (Inspect) Feb 22, 2023 at 7:07PM (UTC)

@tknickman tknickman changed the title feat(convert): initial commit feat(@turbo/workspace-convert): initial commit Feb 15, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@github-actions
Copy link
Contributor

Benchmark for f8cb31d

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9823.58µs ± 87.32µs 9917.25µs ± 102.29µs +0.95%
bench_hmr_to_commit/Turbopack RCC/1000 modules 11.89ms ± 0.13ms 12.22ms ± 0.20ms +2.74%
bench_hmr_to_commit/Turbopack RSC/1000 modules 522.76ms ± 2.74ms 525.24ms ± 2.70ms +0.48%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.26ms ± 0.08ms 10.06ms ± 0.09ms -1.95%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8836.42µs ± 99.16µs 8821.64µs ± 57.33µs -0.17%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.02ms ± 0.16ms 10.83ms ± 0.09ms -1.69%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9009.67µs ± 71.20µs 8933.52µs ± 61.17µs -0.85%
bench_hydration/Turbopack RCC/1000 modules 3920.25ms ± 31.87ms 3875.41ms ± 28.04ms -1.14%
bench_hydration/Turbopack RSC/1000 modules 3438.19ms ± 7.77ms 3432.58ms ± 17.68ms -0.16%
bench_hydration/Turbopack SSR/1000 modules 3585.52ms ± 16.11ms 3623.78ms ± 16.30ms +1.07%
bench_startup/Turbopack CSR/1000 modules 2721.95ms ± 12.59ms 2718.33ms ± 14.12ms -0.13%
bench_startup/Turbopack RCC/1000 modules 2300.52ms ± 4.60ms 2314.40ms ± 5.10ms +0.60%
bench_startup/Turbopack RSC/1000 modules 2219.81ms ± 8.64ms 2249.02ms ± 6.44ms +1.32%
bench_startup/Turbopack SSR/1000 modules 2116.07ms ± 3.21ms 2116.16ms ± 7.20ms +0.00%

@github-actions
Copy link
Contributor

Benchmark for 63d7bc0

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.07ms ± 0.05ms 10.13ms ± 0.08ms +0.63%
bench_hmr_to_commit/Turbopack RCC/1000 modules 11.84ms ± 0.11ms 12.34ms ± 0.15ms +4.24%
bench_hmr_to_commit/Turbopack RSC/1000 modules 517.93ms ± 2.85ms 519.13ms ± 1.77ms +0.23%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.34ms ± 0.07ms 10.34ms ± 0.07ms -0.01%
bench_hmr_to_eval/Turbopack CSR/1000 modules 9032.74µs ± 101.52µs 9205.91µs ± 75.34µs +1.92%
bench_hmr_to_eval/Turbopack RCC/1000 modules 10.68ms ± 0.10ms 10.86ms ± 0.14ms +1.72%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9245.57µs ± 70.70µs 9295.61µs ± 74.87µs +0.54%
bench_hydration/Turbopack RCC/1000 modules 3865.78ms ± 28.21ms 3906.94ms ± 31.37ms +1.06%
bench_hydration/Turbopack RSC/1000 modules 3443.48ms ± 14.03ms 3445.24ms ± 7.93ms +0.05%
bench_hydration/Turbopack SSR/1000 modules 3580.34ms ± 19.51ms 3610.03ms ± 18.81ms +0.83%
bench_startup/Turbopack CSR/1000 modules 2693.96ms ± 6.66ms 2680.74ms ± 9.51ms -0.49%
bench_startup/Turbopack RCC/1000 modules 2301.13ms ± 5.04ms 2306.07ms ± 4.47ms +0.21%
bench_startup/Turbopack RSC/1000 modules 2245.75ms ± 6.55ms 2223.80ms ± 5.30ms -0.98%
bench_startup/Turbopack SSR/1000 modules 2114.51ms ± 4.13ms 2108.58ms ± 2.23ms -0.28%

Copy link
Contributor

@nathanhammond nathanhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part one.

packages/turbo-test-utils/src/useFixtures.ts Outdated Show resolved Hide resolved
packages/turbo-test-utils/src/useFixtures.ts Outdated Show resolved Hide resolved
packages/turbo-test-utils/src/useFixtures.ts Outdated Show resolved Hide resolved
packages/turbo-utils/src/managers.ts Outdated Show resolved Hide resolved
packages/turbo-utils/src/managers.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/cli.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/cli.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/cli.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/cli.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/cli.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@nathanhammond nathanhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part two.

packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@nathanhammond nathanhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't reviewed pnpm.ts, npm.ts, and yarn.ts. I'll get that on round two.

packages/turbo-workspace-convert/src/install.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/index.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/convert.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/logger.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mehulkar mehulkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any strong objections to this, but couple questions:

  • Why are we making this package? I didn't see any chatter about it, so I'm curious. I'm under the impression that switching package managers is pretty easy and in user-land, so unless I'm missing some nuances, do we really want to maintain another 3,000 lines of code?

  • Nathan mentioned this inline in one place too, but for the external API, this reads a bit better to me:

    # --target flag instead of --npm, et al
    # convert command instead of part of the package name
    npx @turbo/workspaces convert --target=pnpm

    Also leaves open the opportunity to do more things with Workspaces in the future, which I can imagine more use cases for.


describe("clean", () => {
test.each([
[true, true],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

named keys in an object would be easier to read

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Jest does support something similar to this, but it's pretty buggy from what I found (the name replacement wasn't working - and there's a few issues about it as well)

packages/turbo-workspace-convert/src/logger.ts Outdated Show resolved Hide resolved
@tknickman
Copy link
Member Author

tknickman commented Feb 17, 2023

@mehulkar thanks for the review! Curious what you would suggest instead of a package? Just keep it in create-turbo? Even in that case we'll need to maintain the same code (with the exception of the cli file), so why not do it in its own package where we have a clear separation of concerns, and it's made available as utils and a cli that could be useful for others (and us) as well and act as promo for turbo. There's also a lot in here that we will consume from other packages (for example, codemod can use the package mangaer detection utils, eslint-plugin can use the workspace detect utils as well).

Also, out whole ethos is keeping your monorepo broken into small, distinct parts that are easily testable, buildable, and cacheable.

I did at one point consider putting this in codemod, which I could still be convinced of, but I think it also makes a nice package on its own - and packages are cheap so that was my first thought.

Copy link
Member Author

@tknickman tknickman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nathanhammond addressed, going to clean up some of the test matrix generation in a second pass, but everything else should be much cleaner.

packages/turbo-test-utils/src/useFixtures.ts Outdated Show resolved Hide resolved
packages/turbo-test-utils/src/useFixtures.ts Outdated Show resolved Hide resolved
packages/turbo-utils/src/managers.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/README.md Outdated Show resolved Hide resolved
import fs from "fs-extra";
import { existsSync } from "fs-extra";

const pnpm = MANAGERS.pnpm;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, pnpm tends to be it's own special snowflake for a few cases. But I'll look into consolidating


describe("clean", () => {
test.each([
[true, true],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Jest does support something similar to this, but it's pretty buggy from what I found (the name replacement wasn't working - and there's a few issues about it as well)

packages/turbo-workspace-convert/src/install.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspace-convert/src/utils.ts Outdated Show resolved Hide resolved
@tknickman tknickman changed the title feat(@turbo/workspace-convert): initial commit feat(@turbo/workspace): initial commit Feb 22, 2023
@github-actions
Copy link
Contributor

Benchmark for dc1d1f7

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.17ms ± 0.08ms 10.16ms ± 0.09ms -0.09%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.73ms ± 0.42ms 13.12ms ± 0.26ms +3.06%
bench_hmr_to_commit/Turbopack RSC/1000 modules 509.29ms ± 2.13ms 508.86ms ± 1.01ms -0.08%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.21ms ± 0.09ms 10.21ms ± 0.07ms -0.02%
bench_hmr_to_eval/Turbopack CSR/1000 modules 9179.36µs ± 102.54µs 9184.83µs ± 61.40µs +0.06%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.10ms ± 0.19ms 11.03ms ± 0.12ms -0.64%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9224.39µs ± 87.23µs 9272.83µs ± 80.77µs +0.53%
bench_hydration/Turbopack RCC/1000 modules 3755.29ms ± 15.09ms 3757.10ms ± 10.51ms +0.05%
bench_hydration/Turbopack RSC/1000 modules 3416.33ms ± 8.20ms 3400.79ms ± 10.62ms -0.45%
bench_hydration/Turbopack SSR/1000 modules 3446.39ms ± 12.67ms 3435.73ms ± 10.41ms -0.31%
bench_startup/Turbopack CSR/1000 modules 2712.28ms ± 10.16ms 2712.93ms ± 9.89ms +0.02%
bench_startup/Turbopack RCC/1000 modules 2301.87ms ± 4.27ms 2316.97ms ± 4.42ms +0.66%
bench_startup/Turbopack RSC/1000 modules 2230.83ms ± 6.30ms 2229.22ms ± 5.05ms -0.07%
bench_startup/Turbopack SSR/1000 modules 2096.83ms ± 2.12ms 2095.68ms ± 1.62ms -0.06%

Copy link
Contributor

@nathanhammond nathanhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part one, 42/61.

packages/turbo-utils/src/managers.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/README.md Outdated Show resolved Hide resolved
"ui": "*"
},
"devDependencies": {
"tsconfig": "*"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found our code that looks for the alternative works-but-not-documented package.json structure:

https://github.com/vercel/turbo/blob/c1ac2ee028e3053dbeb3bf40d0a310afe8f3147c/crates/turborepo-lib/src/package_manager.rs#L146-L155

This is something we need to fix (and we can normalize to the current state).

packages/turbo-workspaces/README.md Outdated Show resolved Hide resolved
packages/turbo-workspaces/README.md Outdated Show resolved Hide resolved
packages/turbo-workspaces/README.md Outdated Show resolved Hide resolved
packages/turbo-workspaces/README.md Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/cli.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/commands/summary/types.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/convert.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/getWorkspaceDetails.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/index.ts Show resolved Hide resolved
packages/turbo-workspaces/src/utils.ts Outdated Show resolved Hide resolved
try {
return fs.readJsonSync(packageJsonPath, "utf8");
} catch (_) {
throw new ConvertError(`no "package.json" found at ${workspaceRoot}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error can be either ENOENT or JSON.parse.

packages/turbo-workspaces/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/utils.ts Outdated Show resolved Hide resolved
packages/turbo-workspaces/src/utils.ts Show resolved Hide resolved
@github-actions
Copy link
Contributor

Benchmark for ac1b67c

Test Base PR % Significant %
bench_hmr_to_eval/Turbopack RCC/1000 modules 12.07ms ± 0.18ms 11.10ms ± 0.14ms -8.06% -2.75%
bench_hydration/Turbopack RCC/1000 modules 3776.59ms ± 6.92ms 3942.79ms ± 25.12ms +4.40% +2.69%
bench_hydration/Turbopack RSC/1000 modules 3427.82ms ± 12.45ms 3599.64ms ± 12.41ms +5.01% +3.54%
bench_hydration/Turbopack SSR/1000 modules 3352.19ms ± 11.67ms 3598.47ms ± 8.37ms +7.35% +6.11%
bench_startup/Turbopack CSR/1000 modules 2608.01ms ± 9.47ms 2864.61ms ± 19.16ms +9.84% +7.59%
bench_startup/Turbopack RCC/1000 modules 2331.39ms ± 3.14ms 2396.11ms ± 4.10ms +2.78% +2.15%
bench_startup/Turbopack RSC/1000 modules 2266.73ms ± 4.70ms 2311.74ms ± 8.93ms +1.99% +0.78%
bench_startup/Turbopack SSR/1000 modules 2053.71ms ± 2.22ms 2173.40ms ± 2.34ms +5.83% +5.37%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.02ms ± 0.09ms 10.13ms ± 0.08ms +1.14%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.89ms ± 0.40ms 12.52ms ± 0.28ms -2.87%
bench_hmr_to_commit/Turbopack RSC/1000 modules 505.71ms ± 2.57ms 516.10ms ± 2.78ms +2.05%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9904.22µs ± 110.66µs 9982.30µs ± 93.21µs +0.79%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8943.78µs ± 69.71µs 8935.05µs ± 88.26µs -0.10%
bench_hmr_to_eval/Turbopack RCC/1000 modules 12.07ms ± 0.18ms 11.10ms ± 0.14ms -8.06% -2.75%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8939.69µs ± 99.43µs 8962.35µs ± 51.15µs +0.25%
bench_hydration/Turbopack RCC/1000 modules 3776.59ms ± 6.92ms 3942.79ms ± 25.12ms +4.40% +2.69%
bench_hydration/Turbopack RSC/1000 modules 3427.82ms ± 12.45ms 3599.64ms ± 12.41ms +5.01% +3.54%
bench_hydration/Turbopack SSR/1000 modules 3352.19ms ± 11.67ms 3598.47ms ± 8.37ms +7.35% +6.11%
bench_startup/Turbopack CSR/1000 modules 2608.01ms ± 9.47ms 2864.61ms ± 19.16ms +9.84% +7.59%
bench_startup/Turbopack RCC/1000 modules 2331.39ms ± 3.14ms 2396.11ms ± 4.10ms +2.78% +2.15%
bench_startup/Turbopack RSC/1000 modules 2266.73ms ± 4.70ms 2311.74ms ± 8.93ms +1.99% +0.78%
bench_startup/Turbopack SSR/1000 modules 2053.71ms ± 2.22ms 2173.40ms ± 2.34ms +5.83% +5.37%

@tknickman tknickman added the pr: automerge Kodiak will merge these automatically after checks pass label Feb 22, 2023
@tknickman
Copy link
Member Author

tknickman commented Feb 22, 2023

Updated, shipping this as first version to keep the project moving and will address further feedback in followups

@tknickman tknickman changed the title feat(@turbo/workspace): initial commit feat(@turbo/workspaces): add new workspaces package (#3818 Feb 22, 2023
@tknickman tknickman merged commit 55f7060 into main Feb 22, 2023
@tknickman tknickman deleted the turbo-convert branch February 22, 2023 19:06
@github-actions
Copy link
Contributor

Benchmark for 53fd1d4

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 12.40ms ± 0.11ms 12.37ms ± 0.09ms -0.22%
bench_hmr_to_commit/Turbopack RCC/1000 modules 16.65ms ± 0.29ms 16.42ms ± 0.18ms -1.35%
bench_hmr_to_commit/Turbopack RSC/1000 modules 512.17ms ± 1.85ms 511.50ms ± 1.45ms -0.13%
bench_hmr_to_commit/Turbopack SSR/1000 modules 12.36ms ± 0.09ms 12.44ms ± 0.11ms +0.62%
bench_hmr_to_eval/Turbopack CSR/1000 modules 11.43ms ± 0.14ms 11.49ms ± 0.15ms +0.47%
bench_hmr_to_eval/Turbopack RCC/1000 modules 15.26ms ± 0.30ms 15.06ms ± 0.30ms -1.28%
bench_hmr_to_eval/Turbopack SSR/1000 modules 11.30ms ± 0.11ms 11.35ms ± 0.10ms +0.39%
bench_hydration/Turbopack RCC/1000 modules 3829.48ms ± 11.44ms 3869.32ms ± 17.32ms +1.04%
bench_hydration/Turbopack RSC/1000 modules 3505.58ms ± 8.69ms 3521.95ms ± 15.08ms +0.47%
bench_hydration/Turbopack SSR/1000 modules 3372.36ms ± 10.76ms 3394.72ms ± 14.48ms +0.66%
bench_startup/Turbopack CSR/1000 modules 2578.72ms ± 5.94ms 2581.07ms ± 6.21ms +0.09%
bench_startup/Turbopack RCC/1000 modules 2367.53ms ± 5.04ms 2364.73ms ± 6.66ms -0.12%
bench_startup/Turbopack RSC/1000 modules 2309.49ms ± 10.09ms 2307.11ms ± 7.66ms -0.10%
bench_startup/Turbopack SSR/1000 modules 2074.27ms ± 2.87ms 2078.37ms ± 4.30ms +0.20%

Comment on lines +175 to +177
if (!options?.dry) {
fs.rmSync(project.paths.lockfile, { force: true });
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know who caught that one, but good catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: turbo-codemod pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants