`"importHelpers": true` combined with `"module": "preserve"` results in undefined helper usage

[clydin]

🔎 Search Terms

importHelpers, module, preserve

🕗 Version & Regression Information

Fails with TS2354 in versions 5.4.x.
Generates invalid code in >= 5.5 including nightly.

⏯ Playground Link

https://www.typescriptlang.org/play/?esModuleInterop=false&declaration=false&importHelpers=true&target=9&jsx=0&module=200#code/FAMwrgdgxgLglgewgAgKIA8CGBbADgGwFMARQqBAJ0xkoAoBKALmUwgE9kBvAX2GAAEMOAiTKVqdesELpclGMij5MAZxXIAglz7BuQA

💻 Code

function ExampleDecorator(): any {}

@ExampleDecorator()
export class A {}

🙁 Actual behavior

import * as tslib_1 from "tslib";
function ExampleDecorator() { }
let A = (() => {
    let _classDecorators = [ExampleDecorator()];
    let _classDescriptor;
    let _classExtraInitializers = [];
    let _classThis;
    var A = class {
        static { _classThis = this; }
        static {
            const _metadata = typeof Symbol === "function" && Symbol.metadata ? Object.create(null) : void 0;
            __esDecorate(null, _classDescriptor = { value: _classThis }, _classDecorators, { kind: "class", name: _classThis.name, metadata: _metadata }, null, _classExtraInitializers);
            A = _classThis = _classDescriptor.value;
            if (_metadata) Object.defineProperty(_classThis, Symbol.metadata, { enumerable: true, configurable: true, writable: true, value: _metadata });
            __runInitializers(_classThis, _classExtraInitializers);
        }
    };
    return A = _classThis;
})();
export { A };

🙂 Expected behavior

import * as tslib_1 from "tslib";
function ExampleDecorator() { }
let A = (() => {
    let _classDecorators = [ExampleDecorator()];
    let _classDescriptor;
    let _classExtraInitializers = [];
    let _classThis;
    var A = class {
        static { _classThis = this; }
        static {
            const _metadata = typeof Symbol === "function" && Symbol.metadata ? Object.create(null) : void 0;
            tslib_1.__esDecorate(null, _classDescriptor = { value: _classThis }, _classDecorators, { kind: "class", name: _classThis.name, metadata: _metadata }, null, _classExtraInitializers);
            A = _classThis = _classDescriptor.value;
            if (_metadata) Object.defineProperty(_classThis, Symbol.metadata, { enumerable: true, configurable: true, writable: true, value: _metadata });
            tslib_1.__runInitializers(_classThis, _classExtraInitializers);
        }
    };
    return A = _classThis;
})();
export { A };

Additional information about the issue

The namespace import identifier for tslib appears to not be used when generating the helper calls.

The baseline reference also appears to demonstrate the issue:

TypeScript/tests/baselines/reference/importHelpersVerbatimModuleSyntax.js

Lines 48 to 53 in 497316f

	import * as tslib_1 from "tslib";
	function foo(args) {
	const { bar } = args, extraArgs = __rest(args, ["bar"]);
	return extraArgs;
	}
	module.exports = foo;

[rbuckton]

It looks like this hasn't worked since --module preserve was added.

@andrewbranch in createExternalHelpersImportDeclarationIfNeeded we always return an ESM import-style declaration, even though we always use tslib's CommonJS exports. Would you expect under --module preserve that we continue to inject import ... from "tslib", or that we emit it as a require call?

If we go with require, I can modify createExternalHelpersImportDeclarationIfNeeded to return an ImportEqualsDeclaration and have the es module transformer handle the downleveling to require. If we stick with import then I'll need to modify createExternalHelpersImportDeclarationIfNeeded to treat Preserve like it does ESNext.

[andrewbranch]

Hmm, I guess I would expect an ImportEqualsDeclaration if the file extension is .cts, ImportDeclaration otherwise (this should be reflected by program.getImpliedNodeFormatForEmit in that module mode).

even though we always use tslib's CommonJS exports

I would also expect module resolution to work correctly once the synthetic import has the correct syntax—i.e., if we generate an ESM import, module resolution in --module preserve --moduleResolution bundler should set the import condition, because it can see that the emitted syntax will be an import. Vice versa if the synthetic import is an ImportEqualsDeclaration.

[rbuckton]

getImpliedNodeFormatForEmit doesn't detect "type": "commonjs" or "type": "module" for a .ts file when using preserve because getImpliedNodeFormatForFileWorker never hits the branch that populates sourceFile.packageJsonCache.

[rbuckton]

I'm mostly tempted to make it always import= under preserve because it is consistent and the transformer for preserve already properly handles import=. Using the implied node format means that a .ts file in a package with "type": "commonjs" ends up with invalid import syntax.

[rbuckton]

Though, I suppose preserve really isn't meant to work with "type": "commonjs" since we don't have a CJS-only variant for export class et al.

[andrewbranch]

getImpliedNodeFormatForEmit doesn't detect "type": "commonjs" or "type": "module" for a .ts file when using preserve because getImpliedNodeFormatForFileWorker never hits the branch that populates sourceFile.packageJsonCache.

Yeah, this is expected behavior. After community feedback, we decided that .mts and .cts should be universal signals of module format, but package.json "type" should only be used inside node_modules or --module nodenext.