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

chore: explicitly set tsBuildInfoFile in tsconfig.*.json files #409

Merged
merged 2 commits into from
Dec 24, 2023

Conversation

haoqunjiang
Copy link
Member

To avoid polluting the root directory with tsconfig.*.tsbuildinfo files.

outDir in the base config doesn't work because of vuejs/tsconfig#27 (comment) Besides, we need this explicit config in tsconfig.node.json anyways, as it doesn't extend from @vue/tsconfig.

Using .tmp instead of .cache here to better indicate the purpose.

To avoid polluting the root directory with tsconfig.*.tsbuildinfo files.

`outDir` in the base config doesn't work because of vuejs/tsconfig#27 (comment)
Besides, we need this explicit config in `tsconfig.node.json` anyways,
as it doesn't extend from `@vue/tsconfig`.

Using `.tmp` instead of `.cache` here to better indicate the purpose.
@bmulholland
Copy link

Is it worth adding comments to explain what the lines in tsconfig are doing and why?

@haoqunjiang
Copy link
Member Author

I thought the option name and the path were self-explanatory enough?

@haoqunjiang
Copy link
Member Author

and why

That's worth adding to every option in tsconfig files, though. Let's do it in a separate PR.

@@ -4,7 +4,8 @@
"exclude": ["src/**/__tests__/*"],
"compilerOptions": {
"composite": true,
"noEmit": true,
"tsBuildInfoFile": "./node_modules/.tmp/tsconfig.app.tsbuildinfo",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: empty line should be remove? Here and in other configs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to distinguish the emitting-related options from other options… The empty lines would make more sense once we add more detailed explanations in the configs.

template/tsconfig/nightwatch/nightwatch/tsconfig.json Outdated Show resolved Hide resolved
@haoqunjiang haoqunjiang merged commit b1505e4 into main Dec 24, 2023
@haoqunjiang haoqunjiang deleted the chore-tsbuildinfofile branch December 24, 2023 08:58
@segevfiner
Copy link

@sodatea Just wondering, what's the reason for removing noEmit from those tsconfig.json files?

@codethief
Copy link

@segevfiner I suppose because TSC project references require emitting declarations, compare my comment here.

@haoqunjiang
Copy link
Member Author

@segevfiner:

@sodatea Just wondering, what's the reason for removing noEmit from those tsconfig.json files?

😅 I ran into the same question when reviewing my own code… I should've left more comments in the commit…

It's because noEmit: true is turned on by default in @vue/tsconfig: https://github.com/vuejs/tsconfig/blob/480cd96cfeea29125c72d7e31fc18b3cbf11b93f/tsconfig.json#L3-L5
So, I removed it from the project template to reduce the user-facing configuration options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants