Cache is not cleared when `transformer` changes

[oscard0m]

What happens and why it is wrong

Without clean: true option, the plugin is catching the configuration even if changes at rollup config level are done (changing the implementation of a transformer implementation for instance)

Until verbosity: 3 was not set, it was not discovered cache could be an issue

Questions

• Is there any historical reason to set clean: false as default? It sounds like the default configuration should be without """magic""" on top right?
• What do you think about adding a warning when cache is used so users notice about it with default verbosity level?
• I did not check how cache is implemented but, is it an expected behaviour to not clean the cache when there are changes on the transformers passed to this plugin or is a bug?

Environment

macOS catalina v10.15.4
node 14.2.0

Versions

• typescript: 3.9.3
• rollup: 2.10.5
• rollup-plugin-typescript2: 0.27.1

rollup.config.js

Not relevant

tsconfig.json

Not relevant

package.json

Not relevant

plugin output with verbosity 3

Not relevant

[ezolenko]

This is a bug, the cache is supposed to catch rollup config changes -- the whole rollup config object is hashed and used as a part of cache key. Could you post your rollup config and what kind of change did you make? (or better yet, make a small repo with reproduction).

Thanks.

[ezolenko]

Re warning about cache use, which output message made you realize cache might be involved? Maybe I need to print it on other verbosity levels...

[oscard0m]

This is a bug, the cache is supposed to catch rollup config changes -- the whole rollup config object is hashed and used as a part of cache key. Could you post your rollup config and what kind of change did you make? (or better yet, make a small repo with reproduction).

Thanks.

Sorry for the delay in the response. Here is a simplification of the problem:

import { rollup } from 'rollup';
import typescript from 'rollup-plugin-typescript2';
import ts from 'typescript';

const nodeRemover = context => {
  return sourceFile => {
    const visitor = (node) => {
      if (ts.isImportDeclaration(node)) {
        return undefined;
      }

      return ts.visitEachChild(node, visitor, context);
    };

    return ts.visitNode(sourceFile, visitor);
  };
};

const transformer = () => ({
	before: [nodeRemover],
	after: []
});

rollup({
	input: './index.ts',
	plugins: [
		typescript({
			transformers: [transformer]
		})
	]
}).then(bundle => {
	bundle.write({
		file: 'bundle.js'
	})
})

index.ts

// @ts-ignore
import { a } from './src/a';

const b = 'b';

console.log(a);
console.log(b);

---

When modifying the transformer for this:

...
if (ts.isImportDeclaration(node) || ts.isVariableStatement(node)) {
...

even the original index.ts content did not change, the output will change due to the changes in the transformer. Seems that the plugin is not aware of transformer changes and only watches for the files to be transpiled for applying cache policies.

---

When adding clean: true option, it solves the issue.

[oscard0m]

Re warning about cache use, which output message made you realize cache might be involved? Maybe I need to print it on other verbosity levels...

cache hit and cache miss

[Artur93gev]

Same issue. I'm using typescript-transformer-jsx-remove-attributes transformer.
Need to apply it on test build, but not for production one. So I think the cache is still existing across multiple runs of bundler(rollup in my case).

Thanks to @dominguezcelada . clean: true option solved the issue for me.

[oscard0m]

Hi @ezolenko I'm interested in following up this issue as a chance to get involved in the project if possible. What do you think could be the approach to be followed here?

Once agreed on that I would like to draft a PR for it :)

[ezolenko]

Sorry for the delay, if you want to look into it, check if rollup config changes are reflected in this.rollupConfig here:

rollup-plugin-typescript2/src/tscache.ts

Lines 119 to 129 in 8ec49c7

	this.cacheDir = `${this.cacheRoot}/${this.cachePrefix}${hash(
	{
	version: this.cacheVersion,
	rootFilenames,
	options: this.options,
	rollupConfig: this.rollupConfig,
	tsVersion: tsModule.version,
	},
	this.hashOptions,
	)}`;
	}

And if they are, if they change output of hash.

object-hash can't handle async functions for example. Maybe transformers are being wrapped in async by rollup. Or object-hash doesn't like something else.

The fix might be in object-hash or in rollup too.

[oscard0m]

Sorry for the delay, if you want to look into it, check if rollup config changes are reflected in this.rollupConfig here:

When printing the output of rollupConfig I see the first level of properties are functions, not objects:

{
  name: 'rpt2',
  options: [Function: options],
  watchChange: [Function: watchChange],
  resolveId: [Function: resolveId],
  load: [Function: load],
  transform: [Function: transform],
  generateBundle: [Function: generateBundle],
  _ongenerate: [Function: _ongenerate],
  _onwrite: [Function: _onwrite]
}

Printing transform (which I guess it ends up using code from transformers option, it's constant as far as I can see, no matter the change/s I apply to my custom transformer function):

Details

        transform(code, id) {
            generateRound = 0; // in watch mode transform call resets generate count (used to avoid printing too many copies of the same error messages)
            if (!filter(id))
                return undefined;
            allImportedFiles.add(normalize(id));
            const contextWrapper = new RollupContext(pluginOptions.verbosity, pluginOptions.abortOnError, this, "rpt2: ");
            const snapshot = servicesHost.setSnapshot(id, code);
            // getting compiled file from cache or from ts
            const result = cache().getCompiled(id, snapshot, () => {
                const output = service.getEmitOutput(id);
                if (output.emitSkipped) {
                    noErrors = false;
                    // always checking on fatal errors, even if options.check is set to false
                    const diagnostics = lodash.concat(cache().getSyntacticDiagnostics(id, snapshot, () => {
                        return service.getSyntacticDiagnostics(id);
                    }), cache().getSemanticDiagnostics(id, snapshot, () => {
                        return service.getSemanticDiagnostics(id);
                    }));
                    printDiagnostics(contextWrapper, diagnostics, parsedConfig.options.pretty === true);
                    // since no output was generated, aborting compilation
                    cache().done();
                    if (lodash.isFunction(this.error))
                        this.error(safe.red(`failed to transpile '${id}'`));
                }
                const references = getAllReferences(id, servicesHost.getScriptSnapshot(id), parsedConfig.options);
                return convertEmitOutput(output, references);
            });
            if (pluginOptions.check) {
                const diagnostics = lodash.concat(cache().getSyntacticDiagnostics(id, snapshot, () => {
                    return service.getSyntacticDiagnostics(id);
                }), cache().getSemanticDiagnostics(id, snapshot, () => {
                    return service.getSemanticDiagnostics(id);
                }));
                if (diagnostics.length > 0)
                    noErrors = false;
                printDiagnostics(contextWrapper, diagnostics, parsedConfig.options.pretty === true);
            }
            if (result) {
                if (result.references)
                    result.references.map(normalize).map(allImportedFiles.add, allImportedFiles);
                if (watchMode && this.addWatchFile && result.references) {
                    if (tsConfigPath)
                        this.addWatchFile(tsConfigPath);
                    result.references.map(this.addWatchFile, this);
                    context.debug(() => `${safe.green("    watching")}: ${result.references.join("\nrpt2:               ")}`);
                }
                if (result.dts) {
                    const key = normalize(id);
                    declarations[key] = { type: result.dts, map: result.dtsmap };
                    context.debug(() => `${safe.blue("generated declarations")} for '${key}'`);
                }
                const transformResult = { code: result.code, map: { mappings: "" } };
                if (result.map) {
                    if (pluginOptions.sourceMapCallback)
                        pluginOptions.sourceMapCallback(id, result.map);
                    transformResult.map = JSON.parse(result.map);
                }
                return transformResult;
            }
            return undefined;
        }

Comparing hashes, it's always the same one so that's why udates are not getting refreshed in cache.

object-hash can't handle async functions for example. Maybe transformers are being wrapped in async by rollup. Or object-hash doesn't like something else.

The fix might be in object-hash or in rollup too.

Printing the properties of this.host I come up with this.host.transformers which contains the array of transformers I was looking for.

---

I did a quick test hashing only host.transformers (I don't know if other properties in host change between executions so... just in case) but still getting the same hash:

objectHash_1(host.transformers, this.hashOptions)

Not sure how this objectHash utility works with functions and so. Maybe some hashOptions we can play with?

Before getting deeper I would like some guidance from your part if possible @ezolenko

---

💡 created a repo example I'm playing with just case you want to use it to do some checks: https://github.com/oscard0m/rollup-plugin-typescript2-example

[oscard0m]

Here is a playground with object-hash and the problem I pointed in my previous comment

[oscard0m]

Hi @ezolenko! Just a friendly follow up on this issue :)

---

EDIT: Hi @ezolenko, I tried to find you in social networks but I did not. I would like to get as contributor of this project so we can move this forward and even participate in other issues or plans for this plugin. Would you be open for that option?

Thanks