fix(create-docusaurus): Fix TS issues on newly initialized sites #10694
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
|
Size Change: +350 B (0%) Total Size: 11.2 MB ℹ️ View Unchanged
|
|
Size Change: +340 B (0%) Total Size: 11.7 MB ℹ️ View Unchanged
|
slorber
commented
Nov 18, 2024
Comment on lines
-14
to
-18
| "types": [ | ||
| "node", | ||
| "@docusaurus/module-type-aliases", | ||
| "@docusaurus/theme-classic" | ||
| ], |
There was a problem hiding this comment.
Hey @Josh-Cena any opinion on this removal?
I thought types added packages for TS to look at, but it turns out it's the opposite and it restricts TS to only look at these types.
https://www.typescriptlang.org/tsconfig/#types
I misunderstood this config initially and not sure why we would use this instead of just having explicit dependencies. 🤔
More historical context about our TS preset is available here:
- https://github.com/tsconfig/bases/blob/main/bases/docusaurus.json
- Add docusaurus tsconfig/bases#27
- Add @docusaurus/theme-classic to compilerOptions.types tsconfig/bases#70
Wondering if this is safe to be considered as a bugfix for v3, or if I should change this later for v4.
The alternative is that our init template has to depend on @docusaurus/theme-classic + @types/node just because of pnpm (maybe yarn berry pnp too but tsc is disabled in CI atm):
There was a problem hiding this comment.
Going to ship this in next patch and consider it a bugfix
Although it's not 100% retro-compatible, I think it's fine to consider that a site that use types without explicitly depending on them is a buggy site, and that it's ok to break TS typechecking for such sites and recommend site owners to add the missing dependencies.
If this was a bad decision, we can revert it.
sabertazimi
added a commit
to sabertazimi/awesome-notes
that referenced
this pull request
Nov 23, 2024
sabertazimi
added a commit
to sabertazimi/awesome-notes
that referenced
this pull request
Nov 23, 2024
* fix(deps): update dependencies (non-major) * fix(website-tsconfig): add docusaurus types manually More details on facebook/docusaurus#10694. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: sabertazimi <[email protected]>
sabertazimi
added a commit
to sabertazimi/bod
that referenced
this pull request
Nov 23, 2024
github-actions bot
added a commit
to sabertazimi/awesome-notes
that referenced
this pull request
Nov 23, 2024
* fix(deps): update dependencies (non-major) * fix(website-tsconfig): add docusaurus types manually More details on facebook/docusaurus#10694. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: sabertazimi <[email protected]> 56f6d70
sabertazimi
added a commit
to sabertazimi/bod
that referenced
this pull request
Nov 23, 2024
* fix(deps): update dependencies (non-major) * fix(website-tsconfig): add docusaurus types manually More details on facebook/docusaurus#10694. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: sabertazimi <[email protected]>
github-actions bot
added a commit
to sabertazimi/bod
that referenced
this pull request
Nov 23, 2024
* fix(deps): update dependencies (non-major) * fix(website-tsconfig): add docusaurus types manually More details on facebook/docusaurus#10694. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: sabertazimi <[email protected]> 6b000de
7 tasks
|
For anyone passing by and seeing TS errors appearing in v3.6.2, check this comment: #10733 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Surprisingly, our e2e test suite only runs TS type-check for yarn berry (node linker only).
CI e2e tests should type-check the newly initialized sites to ensure no site is initialized with any TS errors
Fix #9080
Test Plan
CI