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

Cache behavior is incorrect when using preprocess #59

Closed
evanw opened this issue Jun 25, 2021 · 2 comments
Closed

Cache behavior is incorrect when using preprocess #59

evanw opened this issue Jun 25, 2021 · 2 comments

Comments

@evanw
Copy link

evanw commented Jun 25, 2021

Context: evanw/esbuild#1394. Here's an example:

const fs = require('fs');
const esbuild = require('esbuild');
const sveltePlugin = require('esbuild-svelte');
const { sass } = require('svelte-preprocess-sass');

(async () => {
  fs.writeFileSync(__dirname + '/app.js', 'import x from "./foo.svelte"\nconsole.log(x)');
  fs.writeFileSync(__dirname + '/foo.svelte', '<style lang="sass">@import "./xyz.sass"</style><div class="xyz">foo</div>');

  // Set color to red
  fs.writeFileSync(__dirname + '/xyz.sass', '.xyz\n  color: red');
  const result = await esbuild.build({
    entryPoints: ['app.js'],
    bundle: true,
    incremental: true,
    write: false,
    outfile: 'out.js',
    plugins: [sveltePlugin({
      preprocess: {
        style: sass(),
      },
    })],
    logLevel: 'info',
  });
  console.log(result.outputFiles[1].text);

  // Set color to green
  fs.writeFileSync(__dirname + '/xyz.sass', '.xyz\n  color: green');
  const result2 = await result.rebuild();
  console.log(result2.outputFiles[1].text);

  result.rebuild.dispose();
})();

This should print red followed by green. However, it prints red twice. I assume this is a result of #43. Explicitly adding cache: false is a workaround, and will generate red followed by green.

The problem here is that preprocessors can depend on additional files other than the input file such as when preprocessed CSS code uses @import. When that happens, those additional files must be included in the cache invalidation check. This is described in detail here: https://esbuild.github.io/plugins/#caching-your-plugin.

I can see two ways of fixing this bug:

  1. The easy way to fix this would be to just disable the cache if there is a preprocessor. Obviously that would result in it running more slowly, but it would at least still be correct.

  2. The hard way to fix this would be to use the dependencies returned by the preprocessor during cache invalidation. Specifically, the call to preprocess() here returns a dependencies property in addition to a code property. You would need to at least a) store those paths and the file contents along with the cached value and b) check the contents of those files when deciding later on whether or not the cache entry is valid. Make sure to handle the case where one of the files is missing (i.e. it was deleted), which should invalidate the cache entry.

I don't have a personal stake in fixing this bug. But I noticed that it was tripping people up, so I thought it would be good to report. Feel free to do whatever you want with this report.

@EMH333
Copy link
Owner

EMH333 commented Jun 28, 2021

Thanks for the detailed report! I'll work on fixing this

EMH333 added a commit that referenced this issue Jul 1, 2021
@EMH333
Copy link
Owner

EMH333 commented Jul 3, 2021

This is fixed in v0.5.3 and esbuild-svelte will now properly track dependencies for cache invalidation. Thanks for reporting!

@EMH333 EMH333 closed this as completed Jul 3, 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