roots / bud

High-performance build system that supports SWC, esbuild, and Babel
https://bud.js.org/
MIT License
333 stars 39 forks source link

[bug] `bud-cache` skips assets emitted by plugins #2385

Closed strarsis closed 1 year ago

strarsis commented 1 year ago

Agreement

Describe the issue

bud-cache skips some assets that are emitted by (custom) webpack plugins.

Some plugins also use a webpack loader that collects specific information that is later emitted as asset by the plugin. When bud-cache caches the assets that would be processed by the webpack loaders (including that one used by the plugin) otherwise, the plugin will get empty information collected by the loader, resulting in missing assets.

This would not be an issue when bud-cache also cached the emitted assets by that plugin.

Expected Behavior

bud-cache also caches assets emitted by webpack plugins.

Actual Behavior

bud-cache does not cache assets emitted by webpack plugins, resulting in those assets missing in subsequent builds (with cache enabled and no source changed).

Steps To Reproduce

A minimal, reproducible sample was created to illustrate the issue (based on Sage 10 + bud):

(Initial, uncached build)

(Subsequent, cached builds)

Subsequent build:

The WP Editor Query Plugin used in this sample uses a loader to collect information which it will then use to emit assets. As no loaders are used in subsequent, cached build runs, the plugin will see no information, hence emitting no assets. This would not cause missing assets when bud-cache cached those emitted assets from the uncached/initial run, too.

@kellymears: I can add more information or adjust the sample project if needed.

version

6.12.3

Logs

No response

Configuration

import WpEditorQueryPlugin from 'wp-editor-query-plugin';

/**
 * Compiler configuration
 *
 * @see {@link https://roots.io/docs/sage sage documentation}
 * @see {@link https://bud.js.org/guides/configure bud.js configuration guide}
 *
 * @param {import('@roots/bud').Bud} app
 */
export default async (app) => {

  await app.extensions.add(new WpEditorQueryPlugin())

  app.build.setLoader(
    `editor-extract-loader`,
    WpEditorQueryPlugin.loader
  )

  app.build.setItem(`wp-editor`, {
    loader: `editor-extract-loader`,
    options: {
      include: ['app'],
      adopt: ['editor'],
    },
  })

  app.build.rules.sass.setUse(items => ['minicss', 'css', 'wp-editor', 'postcss', 'resolveUrl', 'sass'])

  /**
   * Application assets & entrypoints
   *
   * @see {@link https://bud.js.org/docs/bud.entry}
   * @see {@link https://bud.js.org/docs/bud.assets}
   */
  app
    .entry('app', ['@scripts/app', '@styles/app'])
    .entry('editor', ['@scripts/editor', '@styles/editor'])
    .assets(['images']);

  /**
   * Set public path
   *
   * @see {@link https://bud.js.org/docs/bud.setPublicPath}
   */
  app.setPublicPath('/app/themes/sage/public/');

  /**
   * Development server settings
   *
   * @see {@link https://bud.js.org/docs/bud.setUrl}
   * @see {@link https://bud.js.org/docs/bud.setProxyUrl}
   * @see {@link https://bud.js.org/docs/bud.watch}
   */
  app
    .setUrl('http://localhost:3000')
    .setProxyUrl('http://example.test')
    .watch(['resources/views', 'app']);

  /**
   * Generate WordPress `theme.json`
   *
   * @note This overwrites `theme.json` on every build.
   *
   * @see {@link https://bud.js.org/extensions/sage/theme.json}
   * @see {@link https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-json}
   */
  app.wpjson
    .set('settings.color.custom', false)
    .set('settings.color.customDuotone', false)
    .set('settings.color.customGradient', false)
    .set('settings.color.defaultDuotone', false)
    .set('settings.color.defaultGradients', false)
    .set('settings.color.defaultPalette', false)
    .set('settings.color.duotone', [])
    .set('settings.custom.spacing', {})
    .set('settings.custom.typography.font-size', {})
    .set('settings.custom.typography.line-height', {})
    .set('settings.spacing.padding', true)
    .set('settings.spacing.units', ['px', '%', 'em', 'rem', 'vw', 'vh'])
    .set('settings.typography.customFontSize', false)
    .useTailwindColors()
    .useTailwindFontFamily()
    .useTailwindFontSize()
    .enable();
};

Relevant .budfiles

No response

kellymears commented 1 year ago

At first glance it looks like the loader needs to declare its dependencies.

For example, this is how bud.js handles extracting @asset directive in blade source files: https://github.com/roots/bud/blob/main/sources/%40roots/blade-loader/src/asset-loader.cts#L15

The dependency has to be added explicitly otherwise there is no relationship established in the graph (the emitted css is treated as unrelated to the originating source files).

Relevant docs:

I'll confirm later and will issue a PR if I'm correct.

kellymears commented 1 year ago

I issued a couple PRs:

The first just adds a single line to the loader which marks it as not supporting caching, which fixes the issue. Maybe kind of brute force but it does the trick. I had trouble with other approaches.

The second is a broader refactor that removes the plugin & store modules in favor of doing all the work in the loader. This is easily cacheable and behaves nominally.

This would be the core of the suggestion:

  postcss([extract])
    .process(source, { from: parsed.base })
    /**
     * Extract the editor specific css from the source file
     * and emit it as a separate file
     */
    .then((extracted) => {
      const emitPath = join(`editor`, parsed.base);
      this.emitFile(emitPath, extracted.toString(), false);

      /**
       * Remove the editor specific css from the source file
       * and return the result
       */
      postcss([remove])
        .process(source, { from: parsed.base })
        .then((result) => {
          callback(null, result.toString());
        });
    });
talss89 commented 1 year ago

Great! I'll have a look at both approaches.

I agree with you, if we can do it all in the loader, then that's the way to go. I'm not sure how 'collecting' rules from multiple modules and emitting one asset will work though.

I really appreciate the time / effort you've put in. Thank you!

kellymears commented 1 year ago

I was assuming modules would be concatenated since by this point sass/postcss has already processed everything, so local import declarations will have already been replaced.

If this is not a good assumption (it might not be) then yeah, my approach will be a little lacking as-is.

talss89 commented 1 year ago

Thanks for the input on this @strarsis and @kellymears. I've rewritten the plugin as a bud extension using the loader-only approach, and simplified things a lot.

It's at https://github.com/talss89/bud-wp-editor-query

From my POV, happy to close this issue. 🍻

strarsis commented 1 year ago

Yes, with the bud plugin everything works great out of the box.