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

[Docs question] Does watch mode enable incremental compilation? #1049

Closed
zaydek opened this issue Mar 24, 2021 · 9 comments
Closed

[Docs question] Does watch mode enable incremental compilation? #1049

zaydek opened this issue Mar 24, 2021 · 9 comments

Comments

@zaydek
Copy link

zaydek commented Mar 24, 2021

In the docs in the second paragraph of this section, it reads:

If you are using the JavaScript or Go API, you can optionally provide a callback that will be called whenever an incremental build has completed. This can be used to do something once the build is complete (e.g. to reload your app in the browser):

require('esbuild').build({
  entryPoints: ['app.js'],
  outfile: 'out.js',
  bundle: true,
  watch: {
    onRebuild(error, result) {
      if (error) console.error('watch build failed:', error)
      else console.error('watch build succeeded:', result)
    },
  },
}).then(result => {
  // Call "stop" on the result when you're done
  result.stop()
})

Does enabling watch also enable incremental compilation? My confusion is because the text reads "you can optionally provide a callback that will be called whenever an incremental build has completed" but there is no incremental flag in the provided example.

Thank you.

@zaydek zaydek changed the title [Docs question] [Docs question] Does watch mode enable incremental compilation Mar 24, 2021
@zaydek zaydek changed the title [Docs question] Does watch mode enable incremental compilation [Docs question] Does watch mode enable incremental compilation? Mar 24, 2021
@evanw
Copy link
Owner

evanw commented Mar 24, 2021

Ah, sorry. Yes watch mode enables incremental compilation.

@zaydek zaydek closed this as completed Mar 24, 2021
@zaydek zaydek reopened this Mar 24, 2021
@zaydek
Copy link
Author

zaydek commented Mar 24, 2021

Thank you! Feel free to close this at your discretion.

@zaydek
Copy link
Author

zaydek commented Mar 24, 2021

@evanw I’m getting different behavior when incremental is not explicitly enabled but watch is. I noticed that rebuild does not actually rebuild my .scss plugin in this case. Related to our prior convo in #808.

I can provide a repro tomorrow if you need.

@evanw
Copy link
Owner

evanw commented Mar 24, 2021

Sure, a repro could be good. Or at least describing what you mean by different behavior.

@evanw
Copy link
Owner

evanw commented Mar 25, 2021

FWIW "incremental" in the docs was meant to describe this behavior: #980 (onRebuild firing on builds other than the first one). That behavior should probably be changed though, and then the docs should also be changed at which point this won't be an issue.

@zaydek
Copy link
Author

zaydek commented Mar 25, 2021

I tried creating a repro and I realized what the problem is. The subtlety escaped me the first time.

Watch mode means esbuild reacts to filesystem events and triggers onRebuild events whereas when incremental is enabled, result.rebuild() trigger rebuilds but does not trigger onRebuild callbacks.

The confusion is what happens when you have both: my expectation was that when watch and incremental are both used, the watch onRebuild callback fires on result.rebuild, but these are actually separate APIs.

My expectation:

const esbuild = require("esbuild")

function sleep(timeoutMS) {
  return new Promise(resolve => setTimeout(resolve, timeoutMS))
}

async function main() {
  const result = await esbuild.build({
    entryPoints: ["in.js"],
    outfile: "out.js",
    incremental: true,
    watch: {
      onRebuild(error, result) {
        console.log("a rebuild event occurred")
      },
    },
  })
  await sleep(1_000)
  result.rebuild() // <- I expected this to trigger watch.onRebuild
  await sleep(1_000)
  result.rebuild() // <- I expected this to trigger watch.onRebuild
}

main()

I tested this with 0.10.0 just to make sure.

I think this is just a situation where the wording was lost on me. I assumed rebuild was an imperative handler to fire onRebuild events.

Just to be clear -- I am totally OK with this behavior. I think if anything the docs should just try to slightly emphasize these differences if they don’t already. Something like: Note: result.rebuild does not trigger watch.onRebuild events; onRebuild events fire in response to filesystem events being watched and polled, whereas result.rebuild is an imperative handler for you to rebuild your entryPoints at your discretion.

So in this case I probably won’t need to use result.rebuild anymore especially because you’ve added the new APIs for watchFile and watchDirs.

FWIW "incremental" in the docs was meant to describe this behavior: #980 (onRebuild firing on builds other than the first one). That behavior should probably be changed though, and then the docs should also be changed at which point this won't be an issue.

I’m actually totally OK with this behavior. This makes sense to me. That being said, if you did provide an imperative handler to manually fire the watch.onRebuild callback, that may help alleviate some concerns. But at the same time, if users just extract the watch.onRebuild callback as a closure they can call at any time, then it doesn’t really matter.

Ah, sorry. Yes watch mode enables incremental compilation.

Because watch.onRebuild is probably going to be the preferred method for invoking rebuilds, I think it would help to describe result.rebuild() as a fallback / imperative API for when you can’t or don’t want to rely on esbuild’s intelligent filesystem polling behavior.

Honestly this is probably just confusion from working with esbuild pre-0.10.0. I don’t know if I would have confused myself this way if I were starting now.

@zaydek zaydek closed this as completed Mar 25, 2021
@zaydek
Copy link
Author

zaydek commented Mar 25, 2021

Ah, sorry. Yes watch mode enables incremental compilation.

Wait you said watch mode enables incremental compilation, but that is only internally, right? I don’t get access to result.rebuild() when watch is enabled.

I think this is why I had to ask because I couldn’t figure out whether incremental was enabled myself.

const esbuild = require("esbuild")

async function main() {
  const result = await esbuild.build({
    entryPoints: ["in.js"],
    outfile: "out.js",
    watch: {
      onRebuild(error, result) {
        console.log("a rebuild event occurred")
      },
    },
  })
  console.log(result.rebuild === undefined) // true
}

main()

@zaydek zaydek reopened this Mar 25, 2021
@evanw
Copy link
Owner

evanw commented Mar 25, 2021

I just meant that watch mode uses incremental compilation. When watch mode is enabled, rebuilds are not done completely from scratch.

I did not mean that watch: true implies incremental: true. Incremental builds require you to call result.rebuild.dispose() when you are done, so it felt inappropriate to automatically imply incremental: true here. I think having to be explicit about incremental: true to get result.rebuild makes more sense. That way the two features are orthogonal.

@zaydek
Copy link
Author

zaydek commented Mar 26, 2021

Ah OK. So watch uses incremental compilation internally whereas incremental gives you access to APIs such as result.rebuild and result.rebuild.dispose and uses incremental compilation. I definitely missed this. So all I really need is watch: true and the new watchFiles and watchDirs features. incremental : true seems like a distraction for my use-case.

It sounds like users shouldn’t reach for incremental just because it exists if they’re also using watch. Watch seems like it solves for the 90% case whereas incremental seems like it solves for the 10% case when you need more fine-grained control over when incremental rebuilds are triggered. It also seems questionable to use watch: true and incremental: true together unless unless you need to trigger extra rebuilds FS polling won’t catch, but in doing so you can’t exclusively rely on the watch.onRebuild callback because it doesn’t react to result.rebuild.

Sorry for the confusion! And thank you.

The tl;dr for someone stumbling on this thread is: use watch: true and don’t worry about incremental: true (because esbuild uses incremental compilation under the hood when watch is enabled). You should only use incremental: true when you need to tell esbuild when to rebuild via result.rebuild() and do cleanup work with result.rebuild.dispose(). You probably don’t want to use both because a) the new watchFiles and watchDirs features (since 0.10.0) helps you communicate filesystem paths that would otherwise be invisible to esbuild and b) watch.onRebuild does not react to result.rebuild.

@zaydek zaydek closed this as completed Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants