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

Feature request: Service instances #814

Closed
ggoodman opened this issue Feb 16, 2021 · 8 comments
Closed

Feature request: Service instances #814

ggoodman opened this issue Feb 16, 2021 · 8 comments

Comments

@ggoodman
Copy link

With the introduction of the more stateful incremental and watch modes, I wonder if we could explore a different spin on the JS API?

The vague idea I have in mind is that a service instance can be created independently of starting a build. Since these service instances would be fully configured on creation, they could provide a reasonable place for a resolve method. This would also mean that a user of the API wouldn't have to distinguish between build and rebuild (and account for the window between initial build start and completion).

Currently, consumers need to differentiate between pre-initial and post-initial build in order to correctly trigger a build at an arbitrary time. This state management can get tricky and IMHO could live closer to the authority on that state.

@evanw
Copy link
Owner

evanw commented Feb 16, 2021

Interesting. I'm actually planning on removing the service API since it's no longer necessary now that there is only ever one underlying child process. My plan was to just have the esbuild.transform[Sync] and esbuild.build[Sync] functions in the next release and remove esbuild.startService.

Currently, consumers need to differentiate between pre-initial and post-initial build in order to correctly trigger a build at an arbitrary time. This state management can get tricky and IMHO could live closer to the authority on that state.

Can you say more about the problem you're trying to solve? And maybe a code example of such an API? I don't think I understand your proposal. As far as I can tell, the current API already seems to fit what you're proposing (with the one difference that creating an instance also does an initial build):

// Create an instance
const instance = await esbuild.build({ ...options, incremental: true })

// Trigger a build whenever you want
const result = await instance.rebuild()

How is doing the initial build problematic?

@zaydek
Copy link

zaydek commented Feb 16, 2021

I'm actually planning on removing the service API since it's no longer necessary now that there is only ever one underlying child process.

Just wanted to add that I appreciate that you’re going to deprecate the service API. I’ve gotten confused previously between all the different ways to invoke esbuild. Having only transform and build looks ideal.

@evanw
Copy link
Owner

evanw commented Feb 16, 2021

Good to know. The context is that it was added quickly in response to a user request and at the time I was unaware of unref() which makes it possible to use a child process without requiring people to call service.stop(). Many thanks to @SalvatorePreviti who contributed this change in #656. Calling service.stop() no longer does anything by the way. I disabled it in preparation for removing it in the next breaking change.

@ggoodman
Copy link
Author

@evanw let me try and explain a bit better now that I'm at a real keyboard.

In a project I'm working on, we need to orchestrate several interrelated builds, each performed by esbuild. For example, in order to bake some preloading metadata into the SSR Renderer build, we need to rebuild the SSR renderer after each successful client build. In some cases, the trigger for rebuild is metadata and not a file on disk so watch: true isn't an ideal option there.

In this project, there are currently 4 interrelated builds whose timing and re-execution must be coordinated.

If I wanted to benefit from incremental: true and watch: true, I would probably need to reach for a state machine. The machine would be able to determine if a revalidation needs to:

  1. Trigger the initial build (ie, initial state). This is an easy case.
  2. Wait for the initial in-progress build and then trigger .rebuild() on the returned object or trigger an initial build in the event of a build error. This is that grey area between initial build and having a reference to .rebuild().
  3. Call `.rebuild(). This is an easy case.

From this perspective, I figured that esbuild would be better suited to implement such a state machine as it would have appropriate references to cancel in-flight builds if necessary and would be able to better handle the grey area between initial state and first successful build.

Now, as I was thinking about this, I was thinking about #641 and how often I've wanted to be able to invoke esbuild's own resolver logic. One of the challenges that I imagine you're thinking about in designing such an API is how to associate a set of plugins and config with calls to .resolve(). Or, "should .resolve() only be available in plugin context?". I thought that if we had a first class 'service instance', it would be an ideal place to expose .resolve() since it would already have a reference to a full configuration.

Here's an idea of how I think it could look:

interface ResolveOptions {
  importer?: string;
  resolveExtensions?: string[];
  // Anything else that might be fun!
}

interface ResolveResult {
  // Not sure how you'd want to handle `browser` fields that mark imports as `false`
  found: boolean;
  path?: string;
}

interface ESBuildServiceInstance {
  // Main API
  build(): Promise<BuildResult>;
  resolve(spec: string, options: ResolveOptions): Promise<ResolveResult>;

  // Event handlers
  onDidCancelBuild(handler: () => void): { dispose(); };
  onWillRebuild(handler: (e: InvalidationEvent) => void): { dispose(); };
  onDidCompleteBuild(handler: (e: BuildResultEvent) => void): { dispose(); };
  onDidFailBuild(handler: (e: BuildFailedEvent) => void): { dispose(); };
}

@SalvatorePreviti
Copy link
Contributor

SalvatorePreviti commented Feb 18, 2021

One temporary way I am using, before better alternatives are available, is to use esbuild.build() and a plugin to extract the resolved path.
It takes around 1 millisecond to run (warm start). A faster and native implementation would be more useful.

const esbuild = require('esbuild')

/**
 * Resolves a module using esbuild module resolution
 *
 * @param {string} id Module to resolve
 * @param {string} [resolveDir] The directory to resolve from
 * @returns {string} The resolved module
 */
async function esbuildResolve(id, resolveDir = process.cwd()) {
  let _resolve
  const resolvedPromise = new Promise((resolve) => (_resolve = resolve))
  return Promise.race([
    resolvedPromise,
    esbuild
      .build({
        sourcemap: false,
        write: false,
        bundle: true,
        format: 'esm',
        logLevel: 'silent',
        platform: 'node',
        stdin: {
          contents: `import ${JSON.stringify(id)}`,
          loader: 'js',
          resolveDir,
          sourcefile: __filename
        },
        plugins: [
          {
            name: 'esbuildResolve',
            setup(build) {
              build.onLoad({ filter: /.*/ }, ({ path }) => {
                id = path
                _resolve(id)
                return { contents: '' }
              })
            }
          }
        ]
      })
      .then(() => id)
  ])
}

@ggoodman
Copy link
Author

@SalvatorePreviti sorry I didn't get back to you sooner. That's a really clever hack. I love it 😁

@evanw
Copy link
Owner

evanw commented Jan 6, 2022

I'm closing this because I don't plan on building the API this way. Another thing to mention is that resolve() is now exposed to plugins: #1881.

@evanw evanw closed this as completed Jan 6, 2022
@ggoodman
Copy link
Author

Looks like the final ask from this issue landed in #2816. ❤️

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

4 participants