-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
refactor(ganalytics, gtag): move options out of themeConfig #5832
Conversation
✔️ [V2] 🔨 Explore the source changes: 9aadb5e 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/618ba37ec4da26000765255f 😎 Browse the preview: https://deploy-preview-5832--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5832--docusaurus-2.netlify.app/ |
a6f0cd7
to
697c4b7
Compare
Size Change: +326 B (0%) Total Size: 882 kB
ℹ️ View Unchanged
|
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, why still draft?
I suggest adding some edits in this doc page too: https://docusaurus.io/docs/presets#docusauruspreset-classic
@@ -22,7 +22,7 @@ declare module '@generated/site-metadata' { | |||
import type {DocusaurusSiteMetadata} from '@docusaurus/types'; | |||
|
|||
const siteMetadata: DocusaurusSiteMetadata; | |||
export default siteMetadata; | |||
export = siteMetadata; |
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.
not sure to understand what's the advantage of this syntax?
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.
This is about how you're able to import it. Because we have esModuleInterop
there's not a visible change. I forgot why I did this change but at least now it's "more correct". export = X
is for module.exports = X
and export default X
is for module.exports.default = X
.
I actually don't know if my changes are correct because I don't know how to test them... Any ideas? |
If you use the Docusaurus prod site locally it should send requests to GA when navigating, if requests are 200 then we can assume it's working |
fcc0c79
to
99da7a6
Compare
LGTM thanks 👍 |
I don't get the argument behind the change, but what is not counterintuitive is having gtag/ganalytics configuration in a completely different place from algolia configuration. Also, if you believe that "counterintuitive to put the plugin options in theme configuration" - why not add it as a separate element of the |
Algolia is a theme ( If you had previously been unaware of the nuance between themes and plugins and just put all of them under As a regular helper on Discord, you can't imagine the confusion from newcomers. I've seen people declaring
I have no idea what you meant. Preset options are directly passed down to individual plugins. When you write: module.exports = {
presets: [
[
'@docusaurus/preset-classic',
{
googleAnalytics: {
trackingID: 'UA-141789564-1',
anonymizeIP: true,
},
},
],
],
}; It's identical in every way to: module.exports = {
plugins: [
[
'@docusaurus/plugin-google-analytics',
{
trackingID: 'UA-141789564-1',
anonymizeIP: true,
},
],
],
}; You are welcome to use an individual plugin as opposed to the preset if the latter makes more sense to you. |
* Upgrade Docusaurus (and plugins) to 2.0.0-beta.15 * Move gtag config out of themeConfig and into preset-classic config Per: facebook/docusaurus#5832
Note the following does not update to the latest beta docusaurus $ yarn upgrade docusaurus docusaurus-plugin-internaldocs-fb --latest Therefore manually set "2.0.0-beta.15" package.json and rerun `yarn upgrade` The docusaurus.config.js change is required in the new version due to: facebook/docusaurus#5832 The website/src/components/GithubLink.jsx change is required due to: facebook/docusaurus#6516
Summary: Update docusaurus to 2.0.0-beta.16 - Moves the `gtag` config, following: ``` [ERROR] Error: The "gtag" field in themeConfig should now be specified as option for plugin-google-gtag. For preset-classic, simply move themeConfig.gtag to preset options. More information at facebook/docusaurus#5832. ``` - Remove our dependence on `react-markdown` (only used for the Help page) to resolve a build error: ``` Module not found: Error: Can't resolve 'path' in '/Users/rob/workspace/metro/website/node_modules/react-markdown/node_modules/vfile' BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default. This is no longer the case. Verify if you need this module and configure a polyfill for it. If you want to include a polyfill, you need to: - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }' - install 'path-browserify' If you don't want to include a polyfill, you can use an empty module like this: resolve.fallback: { "path": false } Module not found: Error: Can't resolve 'path' in '/Users/rob/workspace/metro/website/node_modules/replace-ext' BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default. This is no longer the case. Verify if you need this module and configure a polyfill for it. If you want to include a polyfill, you need to: - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }' - install 'path-browserify' If you don't want to include a polyfill, you can use an empty module like this: resolve.fallback: { "path": false } ``` - Sets `isCloseable: false` (a new docusaurus feature) on our Ukraine banner, in line with React Native Reviewed By: motiz88 Differential Revision: D34614027 fbshipit-source-id: 15a014ac5c821c01e39fa4df9365e1886e2642fb
necessary docusaurus change facebook/docusaurus#5832
Replace the "Metadatas" keyword with "Metadata" facebook/docusaurus#5871 Moved gtag options out of themeConfig facebook/docusaurus#5832
Docusaurus has issued a breaking change in which both `googleAnalytics` and `gtag` plugins should be moved out of the `themeConfig`. This commit does just that. See: facebook/docusaurus#5832
Breaking changes
plugin-google-analytics
orplugin-google-gtag
: you would need to move yourgoogleAnalytics
orgtag
options fromthemeConfig
topreset-classic
configuration.Why?
It has always been very counterintuitive to put the plugin options in theme configuration. It was made so previously due to technical limitations (separation of server data from client data), but now, with a better infrastructure, we can make the migration to make any future user less confused.
Motivation
Fulfill the long-standing todo of moving the options.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Build