Skip to content
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.

Sign up for GitHub

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: load hexo plugin in the theme's package.json #4771

Merged
merged 5 commits into from
Sep 23, 2021
Merged

feat: load hexo plugin in the theme's package.json #4771

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9de5d8
feat(load_plugins): load hexo plugin in the theme's package.json
stevenjoezhang Sep 7, 2021
d363db2
feat(load_plugins): ignore devDependencies in theme package.json
stevenjoezhang Sep 7, 2021
8c2f6c2
test(load_plugins): load hexo plugin in the theme's package.json
stevenjoezhang Sep 21, 2021
e542517
fix: async
stevenjoezhang Sep 21, 2021
d13dd4e
feat: swap hexoModuleList and themeModuleList
stevenjoezhang Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions lib/hexo/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,16 +268,14 @@ class Hexo extends EventEmitter {
return this.database.model(name, schema);
}

resolvePlugin(name) {
const baseDir = this.base_dir;

resolvePlugin(name, basedir) {
try {
// Try to resolve the plugin with the resolve.sync.
return sync(name, { basedir: baseDir });
return sync(name, { basedir });
} catch (err) {
// There was an error (likely the plugin wasn't found), so return a possibly
// non-existing path that a later part of the resolution process will check.
return join(baseDir, 'node_modules', name);
return join(basedir, 'node_modules', name);
}
}

Expand Down
30 changes: 17 additions & 13 deletions lib/hexo/load_plugins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ module.exports = ctx => {
return loadModules(ctx).then(() => loadScripts(ctx));
};

function loadModuleList(ctx) {
const packagePath = join(ctx.base_dir, 'package.json');
function loadModuleList(ctx, basedir) {
const packagePath = join(basedir, 'package.json');

// Make sure package.json exists
return exists(packagePath).then(exist => {
Expand All @@ -24,7 +24,7 @@ function loadModuleList(ctx) {
const deps = Object.keys(json.dependencies || {});
const devDeps = Object.keys(json.devDependencies || {});

return deps.concat(devDeps);
return basedir === ctx.base_dir ? deps.concat(devDeps) : deps;
});
}).filter(name => {
// Ignore plugins whose name is not started with "hexo-"
Expand All @@ -37,22 +37,26 @@ function loadModuleList(ctx) {
if (name.startsWith('@types/')) return false;

// Make sure the plugin exists
const path = ctx.resolvePlugin(name);
const path = ctx.resolvePlugin(name, basedir);
return exists(path);
}).then(modules => {
return Object.fromEntries(modules.map(name => [name, ctx.resolvePlugin(name, basedir)]));
});
}

function loadModules(ctx) {
return loadModuleList(ctx).map(name => {
const path = ctx.resolvePlugin(name);

// Load plugins
return ctx.loadPlugin(path).then(() => {
ctx.log.debug('Plugin loaded: %s', magenta(name));
}).catch(err => {
ctx.log.error({err}, 'Plugin load failed: %s', magenta(name));
return Promise.all([ctx.base_dir, ctx.theme_dir].map(basedir => loadModuleList(ctx, basedir)))
.then(([hexoModuleList, themeModuleList]) => {
return Object.entries(Object.assign(hexoModuleList, themeModuleList));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the plugin defined in the theme will override the one under hexo. The priority in the theme is higher, but the plugin under hexo is added by the user. I think it should have a higher priority.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two different considerations:

  • The priority in the Hexo package.json is higher: If the theme is not maintained, the users will be able to override the outdated plugins defined in theme package.json
  • The priority in the theme is higher: The plugin in Hexo package.json may not be compatible with the theme, and the theme could provide a proper version for the users

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two different considerations:

Yesh, but I prefer the first one. After all, if you use the plugin defined by the theme, you only need to remove the relevant plugins in your package.json. In the second option, I will not be able to cover the plugins inside the theme.

Or take a poll? 😂

})
.map(([name, path]) => {
// Load plugins
return ctx.loadPlugin(path).then(() => {
ctx.log.debug('Plugin loaded: %s', magenta(name));
}).catch(err => {
ctx.log.error({err}, 'Plugin load failed: %s', magenta(name));
});
});
});
}

function loadScripts(ctx) {
Expand Down