refactor(plugin-google-gtag, plugin-google-analytics): migrate packages to TS#5561
refactor(plugin-google-gtag, plugin-google-analytics): migrate packages to TS#5561slorber merged 3 commits intofacebook:mainfrom
Conversation
|
✔️ [V2] 🔨 Explore the source changes: 9417845 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61453e445d60ed000765ecf4 😎 Browse the preview: https://deploy-preview-5561--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5561--docusaurus-2.netlify.app/ |
| return {}; | ||
| } | ||
| return { | ||
| const HTMLTags = { |
There was a problem hiding this comment.
Don't understand, why create new const here, if we can return just object as it was before?
There was a problem hiding this comment.
That doesn't work and spits out a giant "Type not assignable" error... My best guess is due to some inference funkiness
FWIW, I'll just use type assertion
Josh-Cena
force-pushed
the
ganalytics-ts
branch
from
74cc9a5 to
9417845
Compare
slorber
added
the
pr: maintenance
slorber
left a comment
slorber
left a comment
There was a problem hiding this comment.
LGTM thanks
added some comments but we can merge it
|
|
||
| return { | ||
| onRouteUpdate({location}) { | ||
| onRouteUpdate({location}: {location: Location}) { |
There was a problem hiding this comment.
We should probably create a type for js client modules
| gtag?: unknown; // TODO type plugin | ||
| }; | ||
| export type ThemeConfig = import('@docusaurus/theme-common').ThemeConfig & | ||
| import('@docusaurus/plugin-google-analytics').ThemeConfig & |
There was a problem hiding this comment.
even for refactored TS packages, we still want to move those themeConfig to options. This feels overly complex to have to provide plugin in plugin array + config in theme config: configuring things in one place would provide better DX
There was a problem hiding this comment.
We will move them to options in the future, this is just temporary
4 participants
Motivation
More migration, also provided typings to their
ThemeConfigfields.Hopefully this makes refactoring the
themeConfigoptions to plugin options more convenientHave you read the Contributing Guidelines on pull requests?
Yes