chore: upgrade to TS 4.7, compile with NodeNext #7586
Conversation
Josh-Cena
added
the
pr: maintenance
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
|
Size Change: +10 B (0%) Total Size: 801 kB ℹ️ View Unchanged
|
|
Holding off till new version of vfile is released with better type declaration |
Josh-Cena
force-pushed
the
jc/ts-4.7
branch
2 times, most recently
from
a1bf90c to
dc9963c
Compare
| type RawProps = ComponentProps<typeof ${componentName}Type>; | ||
| // ComponentProps is a little buggy, quick fix. | ||
| // https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/60766 | ||
| type Props = unknown extends RawProps ? {} : RawProps; |
There was a problem hiding this comment.
Ideally revert before merge—depends on if DT is open to fixing this.
There was a problem hiding this comment.
cf Discord
We'd rather export our own ComponentProps or WrapperProps type instead of inlining these more complex types
and let's track DefinitelyTyped/DefinitelyTyped#60766, but not sure there will be a fix so soon
Josh-Cena
force-pushed
the
jc/ts-4.7
branch
2 times, most recently
from
f826fa7 to
3691d0e
Compare
| export const useDocsData = (pluginId: string | undefined): GlobalPluginData => | ||
| export const useDocsData = (pluginId?: string): GlobalPluginData => |
There was a problem hiding this comment.
using | undefined is done on purpose to prevent useDocsData() calls: if the pluginId does not matter in a given context, user should be explicit and use useDocsData(undefined)
There was a problem hiding this comment.
The types exposed in the declaration that I just removed had pluginId?: string though 😄 Should I correct this here? I also feel like it's largely unsafe, but if you are not using multi-instance, omitting the ID is very convenient.
There was a problem hiding this comment.
I'd rather keep this | undefined for now, just for us, considering those client APIs are not documented
| type RawProps = ComponentProps<typeof ${componentName}Type>; | ||
| // ComponentProps is a little buggy, quick fix. | ||
| // https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/60766 | ||
| type Props = unknown extends RawProps ? {} : RawProps; |
There was a problem hiding this comment.
cf Discord
We'd rather export our own ComponentProps or WrapperProps type instead of inlining these more complex types
and let's track DefinitelyTyped/DefinitelyTyped#60766, but not sure there will be a fix so soon
| // @ts-expect-error: TODO, we need to make theme-classic have type: module | ||
| import copy from 'copy-text-to-clipboard'; |
There was a problem hiding this comment.
This is specifically because Webpack resolution is neither node nor NodeNext. Webpack allows exports, importing ESM in CJS, importing CJS in ESM, using both require and import in one file, importing .ts extension, importing .js extension... everything.
https://twitter.com/atcb/status/1536831060960419841
https://gist.github.com/andrewbranch/3020c4e24092bd37f7e210d6f050ef26
|
maybe the explicit pluginId is a bit overkill 🤷♂️ we'll see later if it's worth keeping |
Pre-flight checklist
Motivation
This is still a pain in the ass, but at least better than nothing.
I had to add ambient declarations for a few packages (namely,
etaandwebpackbar) that have incompatible package.json manifest formats. I hope they can be fixed soon.I also corrected the format of our own manifests to make sure they can be consumed by external projects compiled with
NodeNext. I'm already having pain with this in my own projects.This can be a step towards #6520.
Test Plan
Test links
Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/
Related issues/PRs