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

Initial notification to watch.onRebuild #980

Closed
mindplay-dk opened this issue Mar 16, 2021 · 10 comments · Fixed by #2816
Closed

Initial notification to watch.onRebuild #980

mindplay-dk opened this issue Mar 16, 2021 · 10 comments · Fixed by #2816
Labels

Comments

@mindplay-dk
Copy link

I added the watch.onRebuild callback to a build-script, and it immediately seemed like the script had just stalled.

At closer examination, it does in fact build - it's just that there's no notification to this callback for the first build.

Can we trigger a notification during the first build on start-up please? (Most build-tools do, afaik.)

Alternatively, if you think there's a real use-case where someone really only wants a notification for all builds except the first one, perhaps consider adding a second, identical callback, e.g. onBuild, which would get called for every build.

(My use-case is I'd like to automatically run my tests after each incremental build - and so, I'd expect the tests to run at startup, without having to first make an arbitrary change to a file to trigger it.)

@zaydek
Copy link

zaydek commented Mar 16, 2021

@mindplay-dk Would you be able to call result.rebuild() yourself immediately, which I believe would kick off the watch.onRebuild event for you?

I think there’s a semantic problem with making onRebuild automatically invoked immediately, plus it would be considered a breaking change.

@mindplay-dk
Copy link
Author

@mindplay-dk Would you be able to call result.rebuild() yourself immediately, which I believe would kick off the watch.onRebuild event for you?

I vaguely recall using a similar hack with another bundler in the past.

This approach results in two builds at start-up, which is probably not what anybody wants.

Just being able to run a test, or do some post-processing after a build, shouldn't require a work-around anyhow.

Another example: if I'm running Babel to produce an IE-friendly version, that needs to happen after every build.

I think there’s a semantic problem with making onRebuild automatically invoked immediately, plus it would be considered a breaking change.

This is why I suggested adding onBuild as an alternative - which should address your concerns about semantics as well.

@zaydek
Copy link

zaydek commented Mar 16, 2021

This approach results in two builds at start-up, which is probably not what anybody wants.

Does it? I would think that when incremental: true is being used, the second build would effectively no-op but would still invoke the callback for you. I could be wrong but that’s why I recommended it.

Anyway this is just my advice. No worries if it doesn’t work for you.

@osdevisnot
Copy link
Contributor

osdevisnot commented Mar 16, 2021

@mindplay-dk the javascript API for esbuild returns a promise, you should just be able to execute your onRebuild on then or finally.

Untested code:

const onRebuild = () => {}

require('esbuild')
	.build({
		// regular esbuild options
		watch: { onRebuild },
	})
	.finally(onRebuild)

I had similar problem with watch mode starting out.

Here is a reference to complete setup with one of our fastify project where I start/restart the node procecss on build or rebuild - https://twitter.com/osdevisnot/status/1357822454027018240

@mindplay-dk
Copy link
Author

Thank you, but frankly I'm meh on both of these solutions - it's not going to be obvious to someone reading the code what you're trying to accomplish, and it's not going to be easy for anyone else to figure out how to achieve this.

Adding an onBuild, in my opinion, is worth it for the semantics - and a place for inline documentation, so folks can find this.

I would have thought doing something on every build was a far more common scenario than doing something on every build except the first one. Like, for that case, I could actually understand having to write code - maintaining a bool variable to check if it's the first build or something. Doing something on the second build is more common? Not in my town. Go figure. 😉

@evanw evanw added the breaking label Mar 25, 2021
@evanw
Copy link
Owner

evanw commented Mar 25, 2021

I can see the argument for wanting this. I think it'd be good to try. I'd like to avoid adding additional options unless they are necessary, so I'd like to try just changing the behavior of onRebuild. However, there's another issue related to this: #1037. I think that would have to be fixed as well to make this scenario really work as expected.

@mindplay-dk
Copy link
Author

mindplay-dk commented Mar 25, 2021

Based on that other issue, it sounds like the initial build needs to be skipped - and then, on startup, treat the very first build as any rebuild. That is, skip the actual 'build" in watch mode, and trigger a "rebuild" on startup. That should give you consistent behavior in watch mode - and would change the meaning of onRebuild to mean "every build", if that's what you want to do anyhow. 👍

@jasonwilliams
Copy link

jasonwilliams commented Jul 24, 2021

In case you need another use case, in VSCode im trying to migrate an extension to use ESBuild. VSCode's tasks has hooks for "background" tasks which are usually watch mode tasks. It needs an initial hook to know that watching has started. With ESbuild i can't do this because there's no message when watch has started, so my task thinks its hanging. onRebuild is great but i need an initial onBuild also.

Reading above and using this it feels like an onBuild hook would make the most sense here, that way those who don't need it can simply not use it, but it can also hold an error if the first build fails. Then subsequent builds can trigger onRebuild (onRebuild can remain as it is)

@jasonwilliams
Copy link

@evanw is it possible to spit out some message when watch mode starts? At least as an interim solution for the above?

@pixelpusher83
Copy link

pixelpusher83 commented Jun 1, 2023

@evanw is it possible to spit out some message when watch mode starts? At least as an interim solution for the above?

I wanted success / error messages but couldn't achieve it through the esbuild api.
As a workaround, I'm using a 3rd party library (node-watch) and doing the printing myself:

// file: esbuild.config.dev.mjs
import chalk from 'chalk';
import * as esbuild from 'esbuild';
import watch from 'node-watch';

const build = async () => {
  try {
    await esbuild.build({ /* your config goes here */});
    console.log(chalk.green('Compiled successfully!'));
  } catch {
    // esbuild will print the error to stdout so no need to do anything here
    // the try statement is here to avoid process exiting on error
  }
};

// call build on start
build();
watch(
  './',
  {
    recursive: true,
    filter(f, skip) {
      // ignore any file outside the "src" folder
      if (f.startsWith('src')) {
        console.log(`File changed: "${f}"`);
        return true;
      }
      return skip;
    },
  },
  build,
);

On successful build:
image
On error:
image

Do note that I'm using build() every time instead of rebuild() which you can get using the context function.

Hope this helps anyone 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants