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

Remove error on redundant isolatedModules/verbatimModuleSyntax combination #53611

Conversation

andrewbranch
Copy link
Member

This was intended to drive clarity, but doesn’t seem worth it since it’s caused a few problems.

Fixes #53601

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 31, 2023
@andrewbranch
Copy link
Member Author

@typescript-bot cherry-pick to release-5.0

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 31, 2023

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

@typescript-bot
Copy link
Collaborator

Hey @andrewbranch, I've opened #53612 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Mar 31, 2023
Component commits:
f0bc7bd Remove error on redundant isolatedModules/verbatimModuleSyntax combination
@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 31, 2023
@jakebailey
Copy link
Member

Was there any code in transformModule that can now go away re: unsetting bad combos?

@fatcerberus
Copy link

fatcerberus commented Mar 31, 2023

Just for posterity, does this error also break the case I mentioned here? (base config with isolatedModules, extending config sets verbatimModuleSyntax) Because if so, there would probably be no workaround in the absence of this PR.

@andrewbranch
Copy link
Member Author

Was there any code in transformModule that can now go away re: unsetting bad combos?

Very little, because the combo with importsNotUsedAsValues and preserveValueImports is still bad and has to be unset. I could have removed the unsetting of isolatedModules but it doesn’t really change anything.

Just for posterity, does this error also break the case I mentioned #53240 (comment)? (base config with isolatedModules, extending config sets verbatimModuleSyntax) Because if so, there would probably be no workaround in the absence of this PR.

That was never an error, IIUC. I was careful to make it an error to specify the old options on top of the new options but not vice versa. You could extend a config with isolatedModules and apply verbatimModuleSyntax because the latter does more than the former.

@fatcerberus
Copy link

That was never an error, IIUC.

thanks, I wouldn’t have suspected there was asymmetry in the implementation between the “base” options and “extended” options - I figured the two configs were just merged to get the effective config, since that seems the most obvious implementation.

GitMurf added a commit to GitMurf/chrome-extension-svelte-typescript-boilerplate that referenced this pull request Apr 5, 2023
… that broke when it is set to true.

See details here on PR that should fix in next release 5.0.4: microsoft/TypeScript#53611

When 5.0.4 comes out, need to flip isolatedModules back to true

Here is details on the issue: microsoft/TypeScript#53601
DanielRosenwasser pushed a commit that referenced this pull request Apr 5, 2023
…e-5.0 (#53612)

Co-authored-by: Andrew Branch <andrew@wheream.io>
@andrewbranch andrewbranch merged commit f4be5b1 into microsoft:main Apr 5, 2023
19 checks passed
@andrewbranch andrewbranch deleted the allow-verbatimModuleSyntax-and-isolatedModules branch April 5, 2023 22:08
drivron pushed a commit to scenari/typescript that referenced this pull request Sep 14, 2023
…to release-5.0 (microsoft#53612)

Co-authored-by: Andrew Branch <andrew@wheream.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When verbatimModuleSyntax: true, allow an explicit isolatedModules: true
6 participants