-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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(v2): site client modules #3545
Conversation
Deploy preview for docusaurus-2 ready! Built with commit 2f835d6 |
const mod = clientModule.__esModule ? clientModule.default : clientModule; | ||
if (mod && mod[lifecycleAction]) { | ||
mod[lifecycleAction](...args); | ||
const lifecycleFunction = |
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.
I'm not familiar with this code, but surely removing the clientModule.__esModule
won't affect it in any way? It was kind of related to Yarn 2 support (not sure)
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 a behavior change that shouldn't break anything, but allows you to write client lifecycle methods as named exports.
Before we had to use this syntax to add the lifecycle to default expor
export default {
onRouteUpdate({location}) {
console.log('onRouteUpdate', {location});
},
};
Now we can also write the following:
export function onRouteUpdate({location}: {location: Location}) {
console.log('onRouteUpdate', {location});
}
Note it's not a documented feature (yet) and kind of broken, so I plan to improve this as part of #3399
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.
BTW both versions still work, and it's the default export that is picked in priority if both are used, so it should be retrocompatible in all cases even if it's not very likely that such code exist in userland
export function onRouteUpdate({location}: {location: Location}) {
console.log('onRouteUpdate', {location});
}
export default {
onRouteUpdate: function onRouteUpdate({location}: {location: Location}) {
console.log('default onRouteUpdate', {location});
},
};
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 for the explanation, so this is safe for merge.
Co-authored-by: Alexey Pyltsyn <lex61rus@gmail.com>
Motivation
As of today, it's a bit boilerplaty to add some global JS to a site, as you need to create a local site plugin to implement getClientModules.
The following option makes it more idiomatic as you can directly pass the client modules as a config attribute:
siteConfig.clientModules = [myGlobalJsPath, myGlobalCssPath]
See also RN website PR: react-native-website-migration/react-native-website@b75a0e6#diff-20b869a62a5a74fb46c979edc17980f6R1-R19
Have you read the Contributing Guidelines on pull requests?
Yes