SourceFile.impliedNodeFormat - misleading doc comment

[craigphicks]

Bug Report

🔎 Search Terms

impliedNodeFormat

#48794

🕗 Version & Regression Information

branch packageJsonWatch

• I was unable to test this on prior versions because:
  • might not expect impliedNodeFormat to be set correctly in earlier versions

⏯ Playground Link

Not possible

💻 Code

The documentation for SourceFile["impliedNodeFormat"] says

        /**
         * When `module` is `Node12` or `NodeNext`, this field controls whether the
         * source file in question is an ESNext-output-format file, or a CommonJS-output-format
         * module. This is derived by the module resolver as it looks up the file, since
         * it is derived from either the file extension of the module, or the containing
         * `package.json` context, and affects both checking and emit.
         *
         * It is _public_ so that (pre)transformers can set this field,
         * since it switches the builtin `node` module transform. Generally speaking, if unset,
         * the field is treated as though it is `ModuleKind.CommonJS`.
         */
        impliedNodeFormat?: ModuleKind.ESNext | ModuleKind.CommonJS;

however, in a unittest is was found that setting

• setting compilerOptions:module to "Node12" did NOT result in "impliedNodeFormat" being set correctly
• setting compilerOptions:moduleResolution to "Node12", while leaving compilerOptions:module unset, DID result in "impliedNodeFormat" being set correctly

🙁 Actual Current In-code Documentation

Need to set compilerOptions:module

It is the in-code documentation that needs to change.

🙂 Actual Behavior

Need to set compilerOptions:moduleResolution

Test

I set up a unittest in a pull to the packageWatchJson branch. #48958 which shows that moduleResolution set to Node12 works correctly.

[craigphicks]

The source code where it happens

    export function getImpliedNodeFormatForFile(fileName: Path, packageJsonInfoCache: PackageJsonInfoCache | undefined, host: ModuleResolutionHost, options: CompilerOptions): ModuleKind.ESNext | ModuleKind.CommonJS | undefined {
        switch (getEmitModuleResolutionKind(options)) {
            case ModuleResolutionKind.Node12:
            case ModuleResolutionKind.NodeNext:
                return fileExtensionIsOneOf(fileName, [Extension.Dmts, Extension.Mts, Extension.Mjs]) ? ModuleKind.ESNext :
                    fileExtensionIsOneOf(fileName, [Extension.Dcts, Extension.Cts, Extension.Cjs]) ? ModuleKind.CommonJS :
                    fileExtensionIsOneOf(fileName, [Extension.Dts, Extension.Ts, Extension.Tsx, Extension.Js, Extension.Jsx]) ? lookupFromPackageJson() :
                    undefined; // other extensions, like `json` or `tsbuildinfo`, are set as `undefined` here but they should never be fed through the transformer pipeline
            default:
                return undefined;
        }

[craigphicks]

I would say it would sufficient to add some documentation to the docs for compilerOptions, module and moduleResolution.

Just a table like

module	moduleResolution	valid
Node12	node	false
empty	Node12	true
Node12	Node12	true

etc.

That would let people know when the didn't need to, or shouldn't, fill in a parameter.
It's not intuitively obvious otherwise.