vercel / turbo

Incremental bundler and build system optimized for JavaScript and TypeScript, written in Rust – including Turbopack and Turborepo.
https://turbo.build
MIT License
25.55k stars 1.74k forks source link

Docs: Watch Mode docs feedback #8317

Open wujekbogdan opened 1 month ago

wujekbogdan commented 1 month ago

What is the improvement or update you wish to see?

The docs say

When your script has a built-in watcher, you likely don't need to use turbo watch. Instead, use your script's built-in watcher and mark the task as long-running using "persistent": true.

It might be misleading.

Is there any context that might help us understand?

This isn't accurate. The watch mode that bundlers (e.g., tsup, esbuild, rollup) come with is only able to watch for changes made to files they're bundling. This is a major issue and the reason why Turbo needed the watch mode that was introduced recently (thank you devs!). I would either remove this from the documentation or add some clarification.

Does the docs page already exist? Please link to it.

https://turbo.build/repo/docs/reference/watch#using-turbo-watch-with-persistent-tasks

michael-land commented 1 month ago

So currently, it cannot watch persistent tasks?

if I have a package built with TSC (not watching), I want the persistent app to reload whenever the packages are rebuilt.

NicholasLYang commented 1 month ago

Currently persistent tasks are not reloaded. That is something that people have asked for in #8164, so we're definitely looking into it! If you don't mind me asking, what's your specific use case for reloading persistent tasks?

We encourage people to keep using the built in watchers for existing tools because often times tool specific watchers have special logic for that tool, for instance vite dev or next dev are optimized for hot reloading, while vite build and next build are not. If a user were to use turbo watch with vite build, it would provide a much worse user experience than just using vite dev directly. This is also why we didn't include reloading persistent tasks, since often persistent tasks have an initial startup time, so reloading them on every change would be a pretty bad user experience. However, people clearly want this feature, so we're looking into it!

michael-land commented 1 month ago

When working with NestJS in a monorepo setup, I want the server to reboot whenever I edit the packages. For example, I need to reload app-1 and app-2 when package-1 changes. As @wujekbogdan mentioned, the watch mode that bundlers come with can only monitor changes made to files within that specific project.

My current workaround is to have a separate compile task and a separate persistent dev task with Nodemon. Additionally, I have separate compile and dev tasks set up. When package code changes, Turbo detects it and triggers the "compile" task for package-1, app-1 and app-2. This updates the app-1 and app-2 dist directory, causing Nodemon to reboot the persistent script. Hopefully, this can be simplified in future versions.

apps/app1/package.json
{
    "script": {
        "compile": "nest build -b swc",
        "dev": "nodemon dist/main.js --watch dist --delay 1",
    }
}

pacakges/package1/package.json
{
    "script": {
        "compile": "tsc",
    }
}

turbo.json
{
    "compile": {
      "dependsOn": ["^compile"],
      "inputs": ["src/**", "tsconfig.json"],
      "outputs": ["dist/**"]
    },
    "dev": {
      "persistent": true
    }
}
pnpm exec turbo watch dev compile
NicholasLYang commented 1 month ago

Gotcha. Yeah that seems to be a common setup. We'll get that fixed ASAP

anthonyshew commented 1 month ago

@michael-land It sounds to me like we already have the behavior you're looking for but the documentation isn't strong enough to help you put the pieces together.

Listing out the requirements I think you're describing as I'm understanding them. Let me know if I'm wrong so I can adjust:

Will this work for what you need?

// turbo.json
{
   "build" {
       "dependsOn": ["^build"],
       "inputs": ["src/**", "tsconfig.json"],
       "outputs": ["dist/**"],
   }
}

Note that I'm not marking any tasks as persistent here. We're going to solely rely on turbo watch to keep processes awake and aware.

// apps/app-1/package.json and apps/app-2/package.json

{
  "scripts": {
    "build": "nest build -b swc",
  }
}
// packages/package1/package.json

{
  "scripts": {
    "build": "tsc"
}

When I use turbo watch build and make an edit in package1, it will rebuild. When that task is done, the app-1 will rebuild and reboot.

Does that sound right?

(For context, when writing the documentation for this feature, I realized turbo watch such a powerful feature that enables so many different patterns that I didn't know what would be confusing to write down and what would be helpful. I chose to keep it simple in the hopes that folks would provide feedback about what they would find more helpful - and here we are.) 😄

michael-land commented 1 month ago

Hi Anthony,

Thanks for getting back to me.

You suggested the configuration nest build -b swc only works if I want to build and rebuild when dependencies change. During development, I also want to run a development server, e.g., nest start --watch.

If you'd like, I can provide a sample setup for reproduction.

anthonyshew commented 1 month ago

In that case, I believe you would do:

// turbo.json
{
   "build" {
       "dependsOn": ["^build"],
       "inputs": ["src/**", "tsconfig.json"],
       "outputs": ["dist/**"],
   },
   "dev" {
       "dependsOn": ["^dev"],
       "inputs": ["src/**", "tsconfig.json"],
       "outputs": ["dist/**"],
   },
   "app1#dev": {
     "dependsOn": [],
     "persistent": true
     // + whatever other configuration you need
   }
}

Note: In a real repo, I would likely use a Package Configuration instead of the <package>#<task> syntax, but using that syntax here for ease-of-reading. Both ways work according to your needs.

// apps/app-1/package.json and apps/app-2/package.json
{
  "scripts": {
    "build": "nest build -b swc",
    "dev": "nest start --watch"
  }
}
// packages/package1/package.json
{
  "scripts": {
    "build": "tsc",
    "dev": "tsc"
}

A couple of things worthy of note here:

Tell me if I got it this time. 😄

michael-land commented 1 month ago

CleanShot 2024-06-05 at 13 59 22

Tell me if I got it this time. 😄

I guess not 🫢. I created a simple repo for reproduction: https://github.com/michael-land/turborepo2-watch

In the video, I expect the app's console.log to reprint with the new value whenever I save.

anthonyshew commented 1 month ago

This appears to be an issue with the Nest.js hot-reloading server not tracing its modules correctly. I made a PR to your reproduction that shows this: https://github.com/michael-land/turborepo2-watch/pull/1

What I'm noticing:

For me, that indicates that the Nest.js hot reloader isn't quite handling what it is meant to, but I admittedly don't know if the Nest.js hot reloader is supposed to handle this case.

For the sake of thoroughness, I tried the first strategy I proposed and changed the dev script in backend to "nest build -b swc && nest start and things are working as I would expect them to.

Overall, it looks like turbo watch is working as intended here (although we do still need to improve the documentation).

michael-land commented 1 month ago

I tried your PR, and Next.js example is working fine (I guess next.js might also watch node_modules or .next folder for hot reloads).

However, I can't get nest build -b swc && nest start to work. I'll wait to see if others have the same issue. I appreciate your help. Thanks!

anthonyshew commented 1 month ago

Apologies, I left out that you'll want to remove the app1#dev configuration. Pushed another commit to show a diff. Once you do that, it will kinda-hot-reload when you hit save on a file in the Nest.js app.

michael-land commented 1 month ago

it works now 🥳

anthonyshew commented 1 month ago

Love to hear it. Thinking about what docs updates I need to make to make this more clear.

wujekbogdan commented 1 month ago

Currently persistent tasks are not reloaded.

Does it mean that if I set up Turbo the way I described here then I'm all set?

turbo watch dev --filter=@my-namespace/my-app

Where the dev task for the app being watched is defined as:

{
  "dev": "tsup --watch --onSuccess 'node dist/index.js'"
}

The build task for all the packages is defined as:

{
  "build": "tsup"
}

And the turbo config is defined as

{
  "pipeline": {
    "build": {
      "dependsOn": ["^build"],
      "outputs": ["dist/**"]
    },
    "dev": {
      "persistent": true,
      "dependsOn": ["^build"],
      "cache": false
    },
  },
}

Based on what you say, it seems that this setup is correct - @my-namespace/my-app dependencies will restart on change, while the dev task will not (as intended) because it's persistent - it will use thetsup watcher.

Or am I wrong? Maybe I need to explicitly include the build task even though it's a dependency of dev?

turbo watch build dev --filter=@my-namespace/my-app
weyert commented 1 month ago

If I understand it correctly, you enforce the rebuild of the project itself, which will trigger the restart?

I still think something like nx watch would be useful too: https://nx.dev/nx-api/nx/documents/watch

anthonyshew commented 1 month ago

@wujekbogdan That sounds to me like it would work. Do you not get the behavior you're looking for?

wujekbogdan commented 3 weeks ago

@anthonyshew

Yes, it works, but it would be good to clarify the following things in the docs: