feat(core): allow plugin lifecycles to return relative paths #6921
Conversation
Josh-Cena
changed the title
|
✔️ [V2] 🔨 Explore the source changes: ca2e6c9 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/62313ed9db172700081991d1 😎 Browse the preview: https://deploy-preview-6921--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6921--docusaurus-2.netlify.app/ |
|
Size Change: 0 B Total Size: 792 kB ℹ️ View Unchanged
|
|
✔️ [V2] 🔨 Explore the source changes: f20cba2 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6231d96c0ae67b0009670d52 😎 Browse the preview: https://deploy-preview-6921--docusaurus-2.netlify.app |
Does this mean that all third-party plugin authors would have to do the same too? Or is it just for our own conversion to ESM? |
| /** | ||
| * The absolute path to the folder containing the entry point file. | ||
| */ | ||
| readonly path: string; |
There was a problem hiding this comment.
maybe having a more explicit name would be better than a comment?
There was a problem hiding this comment.
I think it's explicit enough. The only reason I added the comment is because I would add a comment to all path-related variables in the future. Paths are always ambiguous and it's not really worth making the name super long.
| @@ -8,5 +8,6 @@ | |||
| module.exports = function() { | |||
| return { | |||
| name: 'plugin-empty', | |||
| path: __dirname, | |||
There was a problem hiding this comment.
not sure to understand why it's needed in the fixtures?
Fixtures should rather look like userland code and I don't think we want to ask users to provide this path?
Maybe something like baseResolvePath or something? 🤷♂️ not easy
There was a problem hiding this comment.
The fixture returns a InitializedPlugin, not a raw plugin. It's not strictly a userland fixture, just something that mocks an intermediate value in our loading process 😅 Legacy test, don't want to touch much. I would probably refactor the core code and tests later
| * Different from pluginModule.path, this one is always an absolute path used | ||
| * to resolve relative paths returned from lifecycles | ||
| */ | ||
| path: string; |
There was a problem hiding this comment.
also find a better attribute name?
It's primarily for us, but it also means community plugins can migrate to ESM more easily. They don't have to, though. |
Motivation
This is yet another effort to migrate to ES modules. #6520
Being able to resolve relative paths means we no longer rely on the
__dirnamevariable, which doesn't exist in ESM contexts. It also gives plugin authors more flexibility in how they write their code.Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Converted all plugins to return relative paths