-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(plugin-typescript): check workspace tsconfig.json #6175
feat(plugin-typescript): check workspace tsconfig.json #6175
Conversation
packages/acceptance-tests/pkg-tests-specs/sources/plugins/plugin-typescript.test.ts
Outdated
Show resolved
Hide resolved
?? xfs.existsSync(ppath.join(workspace.cwd, `tsconfig.json`)) | ||
?? xfs.existsSync(ppath.join(project.cwd, `tsconfig.json`)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?? xfs.existsSync(ppath.join(workspace.cwd, `tsconfig.json`)) | |
?? xfs.existsSync(ppath.join(project.cwd, `tsconfig.json`)); | |
?? (xfs.existsSync(ppath.join(workspace.cwd, `tsconfig.json`)) | |
|| xfs.existsSync(ppath.join(project.cwd, `tsconfig.json`))); |
Otherwise it doesn't check the project root as well, if that's on purpose then:
?? xfs.existsSync(ppath.join(workspace.cwd, `tsconfig.json`)) | |
?? xfs.existsSync(ppath.join(project.cwd, `tsconfig.json`)); | |
?? xfs.existsSync(ppath.join(workspace.cwd, `tsconfig.json`)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, definitely a bug, it was causing #6175 (comment). I'm still not sure if I should re-introduce it though. If you're in a monorepo, and do not have a tsconfig.json
in one of your workspaces, it might be safe to assume you're not using TypeScript in that specific workspace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're in a monorepo, and do not have a tsconfig.json in one of your workspaces, it might be safe to assume you're not using TypeScript in that specific workspace
Not necessarily, this repo is an example of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright thanks, that settles it then, it would have been an odd behavior anyway 👍
The integ tests should be good to go, the builds that failed did so on unrelated specs ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What's the problem this PR addresses?
Given the following monorepo setup,
plugin-typescript
is currently not enabled by default in thefoo
workspace, as it only checks that atsconfig.json
exists at the root of the project:I also found the current
README
ofplugin-typescript
not to be helpful in diagnosing my issueHow did you fix it?
I've added a check to see if the current workspace had a
tsconfig.json
I've also added a Configuration section to
plugin-typescript/README.md
, to help users find thetsEnableAutoTypes
option.Checklist
I have set the packages that need to be released for my changes to be effective.
@yarnpkg/plugin-typescript
and@yarnpkg/cli
, let me know if I should also select the other plugins/core etc.