Allow 'verbatimModuleSyntax' with transpileModule

[rbuckton]

Since --verbatimModuleSyntax implies --isolatedModules, it should be allowed in conjunction with transpileModule. However, when both options are specified an error is reported, and transpileModule always explicitly sets isolatedModules with no way to unset the option.

This changes transpileModule to skip any potential redudndant flags when setting defaults for transpileModule.

Fixes #53160

[fatcerberus]

However, when both options are specified an error is reported

I feel like the error should only be reported for verbatimModuleSyntax: true, isolatedModules: false. This error probably also breaks cases where a base config uses isolatedModules and then something extends it with verbatimModuleSyntax

[rbuckton]

However, when both options are specified an error is reported

I feel like the error should only be reported for verbatimModuleSyntax: true, isolatedModules: false. This error probably also breaks cases where a base config uses isolatedModules and then something extends it with verbatimModuleSyntax

extends is related to tsconfig.json/tsc, so while your comment may be valid it might be better addressed in a separate issue. transpileModule is a language service API and has little to do with extends.

[fatcerberus]

Yeah, that second sentence was just me musing. The point was the first sentence. Having both verbatimModuleSyntax and isolatedModules set is not contradictory so probably shouldn't be an error at all (in which case this PR would be unnecessary).

[DanielRosenwasser]

Yes, it feels like there's not exactly a ton of "harm" - you can set checkJs with allowJs too, though checkJs didn't also imply allowJS previously.

[DanielRosenwasser]

Does anyone have any idea why we're getting all these target=ES3 issues in the tests?

[jakebailey]

Probably some unit test default still set to ES3?

[jakebailey]

Probably some unit test default still set to ES3?

Yep, in the test's transpilesCorrectly:

            if (transpileOptions.compilerOptions.target === undefined) {
                transpileOptions.compilerOptions.target = ts.ScriptTarget.ES3;
            }

That probably should have been changed when we changed the default, or something. Hard to say when some tests surely tested things about ES3 support, but, we're not going to change that.

Anyway, probably too late to change this as it'd break cherry-picks.

[DanielRosenwasser]

I don't understand, why didn't all these tests break before?

[jakebailey]

They did, this PR just adds another column to the settings matrix: https://github.com/microsoft/TypeScript/pull/53240/files#diff-15a7ae1852c1cd6c50a9bf7a2453379ef7fce45691dabc656f157272069782b3R94

And then all of the tests below being updated to test that.

[DanielRosenwasser]

@typescript-bot cherry-pick this to release-5.0 and LKG

[typescript-bot]

Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into release-5.0 on this PR at ff7b1de. You can monitor the build here.

[jakebailey]

I don't think either of these can actually show up in transpileOptionValueCompilerOptions because neither explicitly set transpileOptionValue. But, I guess it's harmless if we want this PR in ASAP.

[rbuckton]

I added them just to be on the safe side, since it would be fairly easy to miss if we made any changes in compilerOptions.

[typescript-bot]

Hey @DanielRosenwasser, I've opened #53253 for you.