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

Jump to bottom

refactor(plugins): switched from require to await import() when loading plugins #2558

Merged
merged 6 commits into from Sep 17, 2022
Merged

refactor(plugins): switched from require to await import() when loading plugins #2558

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fbc528b
refactor(plugins): switched from `require` to `await import()` when l…
travi Sep 12, 2022
476e0bb
fix(plugins): referenced the default of the imported plugin
travi Sep 16, 2022
4fb3cd2
refactor(plugins): improved variable names within reduce loops
travi Sep 16, 2022
47ca65c
refactor(plugins): removed console.logs and resolved lint issues
travi Sep 16, 2022
97fdd48
Merge branch 'master' of github.com:semantic-release/semantic-release…
travi Sep 16, 2022
eeae4d3
test(lint): disabled the check for `node/no-unsupported-features/es-s…
travi Sep 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/definitions/errors.js
View file
Open in desktop
Expand Up @@ -68,7 +68,7 @@ Your configuration for the \`${type}\` plugin is \`${stringify(pluginConf)}\`.`,
message: 'The `plugins` configuration is invalid.',
details: `The [plugins](${linkify(
'docs/usage/configuration.md#plugins'
)}) option must be an array of plugin definions. A plugin definition is an npm module name, optionally wrapped in an array with an object.
)}) option must be an array of plugin definitions. A plugin definition is an npm module name, optionally wrapped in an array with an object.

The invalid configuration is \`${stringify(plugin)}\`.`,
}),
Expand Down
72 changes: 42 additions & 30 deletions lib/plugins/index.js
View file
Open in desktop
Expand Up @@ -6,36 +6,41 @@ const {validatePlugin, validateStep, loadPlugin, parseConfig} = require('./utils
const pipeline = require('./pipeline');
const normalize = require('./normalize');

module.exports = (context, pluginsPath) => {
module.exports = async (context, pluginsPath) => {
let {options, logger} = context;
const errors = [];

const plugins = options.plugins
? castArray(options.plugins).reduce((plugins, plugin) => {
if (validatePlugin(plugin)) {
const [name, config] = parseConfig(plugin);
plugin = isString(name) ? loadPlugin(context, name, pluginsPath) : name;
? await castArray(options.plugins).reduce(async (plugins, plugin) => {
const plugins2 = await plugins;
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there is a better name for plugins2? Maybe normalizedPlugins or awaitedPlugins?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, this is definitely part of the clean up that is necessary once this is working :)

if (validatePlugin(plugin)) {
const [name, config] = parseConfig(plugin);
plugin = isString(name) ? await loadPlugin(context, name, pluginsPath) : name;

if (isPlainObject(plugin)) {
Object.entries(plugin).forEach(([type, func]) => {
if (PLUGINS_DEFINITIONS[type]) {
Reflect.defineProperty(func, 'pluginName', {
value: isPlainObject(name) ? 'Inline plugin' : name,
writable: false,
enumerable: true,
});
plugins[type] = [...(plugins[type] || []), [func, config]];
}
});
} else {
errors.push(getError('EPLUGINSCONF', {plugin}));
}
console.log({plugins: options.plugins})
console.dir(plugin, {depth: null})
console.log(isPlainObject(plugin))
Copy link
Member

Choose a reason for hiding this comment

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

just a reminder to remove the debug logs when the PR is ready :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yep 👍🏼

if (isPlainObject(plugin)) {
Object.entries(plugin).forEach(([type, func]) => {
console.log({type, name})
if (PLUGINS_DEFINITIONS[type]) {
Reflect.defineProperty(func, 'pluginName', {
value: isPlainObject(name) ? 'Inline plugin' : name,
writable: false,
enumerable: true,
});
plugins2[type] = [...(plugins2[type] || []), [func, config]];
}
});
} else {
errors.push(getError('EPLUGINSCONF', {plugin}));
}
} else {
errors.push(getError('EPLUGINSCONF', {plugin}));
}

return plugins;
}, {})
return plugins2;
}, {})
: [];

if (errors.length > 0) {
Expand All @@ -44,9 +49,16 @@ module.exports = (context, pluginsPath) => {

options = {...plugins, ...options};

const pluginsConf = Object.entries(PLUGINS_DEFINITIONS).reduce(
(pluginsConf, [type, {required, default: def, pipelineConfig, postprocess = identity, preprocess = identity}]) => {
const pluginsConf = await Object.entries(PLUGINS_DEFINITIONS).reduce(
async (pluginsConf, [type, {
required,
default: def,
pipelineConfig,
postprocess = identity,
preprocess = identity
}]) => {
let pluginOptions;
const pluginsConf2 = await pluginsConf;

if (isNil(options[type]) && def) {
pluginOptions = def;
Expand All @@ -60,28 +72,28 @@ module.exports = (context, pluginsPath) => {

if (!validateStep({required}, options[type])) {
errors.push(getError('EPLUGINCONF', {type, required, pluginConf: options[type]}));
return pluginsConf;
return pluginsConf2;
}

pluginOptions = options[type];
}

const steps = castArray(pluginOptions).map((pluginOpt) =>
normalize(
const steps = await Promise.all(castArray(pluginOptions).map(async (pluginOpt) =>
await normalize(
{...context, options: omit(options, Object.keys(PLUGINS_DEFINITIONS), 'plugins')},
type,
pluginOpt,
pluginsPath
)
);
));

pluginsConf[type] = async (input) =>
pluginsConf2[type] = async (input) =>
postprocess(
await pipeline(steps, pipelineConfig && pipelineConfig(pluginsConf, logger))(await preprocess(input)),
await pipeline(steps, pipelineConfig && pipelineConfig(pluginsConf2, logger))(await preprocess(input)),
input
);

return pluginsConf;
return pluginsConf2;
},
plugins
);
Expand Down
6 changes: 4 additions & 2 deletions lib/plugins/normalize.js
View file
Open in desktop
Expand Up @@ -5,18 +5,20 @@ const {extractErrors} = require('../utils');
const PLUGINS_DEFINITIONS = require('../definitions/plugins');
const {loadPlugin, parseConfig} = require('./utils');

module.exports = (context, type, pluginOpt, pluginsPath) => {
module.exports = async (context, type, pluginOpt, pluginsPath) => {
const {stdout, stderr, options, logger} = context;
if (!pluginOpt) {
return noop;
}

const [name, config] = parseConfig(pluginOpt);
const pluginName = name.pluginName ? name.pluginName : isFunction(name) ? `[Function: ${name.name}]` : name;
const plugin = loadPlugin(context, name, pluginsPath);
const plugin = await loadPlugin(context, name, pluginsPath);

debug(`options for ${pluginName}/${type}: %O`, config);

console.log({plugin})
console.dir(plugin, {depth: null})
let func;
if (isFunction(plugin)) {
func = plugin.bind(null, cloneDeep({...options, ...config}));
Expand Down
4 changes: 2 additions & 2 deletions lib/plugins/utils.js
View file
Open in desktop
Expand Up @@ -44,11 +44,11 @@ function validateStep({required}, conf) {
return conf.length === 0 || validateSteps(conf);
}

function loadPlugin({cwd}, name, pluginsPath) {
async function loadPlugin({cwd}, name, pluginsPath) {
const basePath = pluginsPath[name]
? dirname(resolveFrom.silent(__dirname, pluginsPath[name]) || resolveFrom(cwd, pluginsPath[name]))
: __dirname;
return isFunction(name) ? name : require(resolveFrom.silent(basePath, name) || resolveFrom(cwd, name));
return isFunction(name) ? name : (await import(resolveFrom.silent(basePath, name)) || resolveFrom(cwd, name));
}

function parseConfig(plugin) {
Expand Down
57 changes: 29 additions & 28 deletions test/plugins/normalize.test.js
View file
Open in desktop
Expand Up @@ -19,8 +19,8 @@ test.beforeEach((t) => {
};
});

test('Normalize and load plugin from string', (t) => {
const plugin = normalize(
test('Normalize and load plugin from string', async (t) => {
const plugin = await normalize(
{cwd, options: {}, logger: t.context.logger},
'verifyConditions',
'./test/fixtures/plugin-noop',
Expand All @@ -32,8 +32,8 @@ test('Normalize and load plugin from string', (t) => {
t.deepEqual(t.context.success.args[0], ['Loaded plugin "verifyConditions" from "./test/fixtures/plugin-noop"']);
});

test('Normalize and load plugin from object', (t) => {
const plugin = normalize(
test('Normalize and load plugin from object', async (t) => {
const plugin = await normalize(
{cwd, options: {}, logger: t.context.logger},
'publish',
{path: './test/fixtures/plugin-noop'},
Expand All @@ -45,8 +45,8 @@ test('Normalize and load plugin from object', (t) => {
t.deepEqual(t.context.success.args[0], ['Loaded plugin "publish" from "./test/fixtures/plugin-noop"']);
});

test('Normalize and load plugin from a base file path', (t) => {
const plugin = normalize({cwd, options: {}, logger: t.context.logger}, 'verifyConditions', './plugin-noop', {
test('Normalize and load plugin from a base file path', async (t) => {
const plugin = await normalize({cwd, options: {}, logger: t.context.logger}, 'verifyConditions', './plugin-noop', {
'./plugin-noop': './test/fixtures',
});

Expand All @@ -58,7 +58,7 @@ test('Normalize and load plugin from a base file path', (t) => {
});

test('Wrap plugin in a function that add the "pluginName" to the error"', async (t) => {
const plugin = normalize({cwd, options: {}, logger: t.context.logger}, 'verifyConditions', './plugin-error', {
const plugin = await normalize({cwd, options: {}, logger: t.context.logger}, 'verifyConditions', './plugin-error', {
'./plugin-error': './test/fixtures',
});

Expand All @@ -68,7 +68,7 @@ test('Wrap plugin in a function that add the "pluginName" to the error"', async
});

test('Wrap plugin in a function that add the "pluginName" to multiple errors"', async (t) => {
const plugin = normalize({cwd, options: {}, logger: t.context.logger}, 'verifyConditions', './plugin-errors', {
const plugin = await normalize({cwd, options: {}, logger: t.context.logger}, 'verifyConditions', './plugin-errors', {
'./plugin-errors': './test/fixtures',
});

Expand All @@ -78,16 +78,16 @@ test('Wrap plugin in a function that add the "pluginName" to multiple errors"',
}
});

test('Normalize and load plugin from function', (t) => {
test('Normalize and load plugin from function', async (t) => {
const pluginFunction = () => {};
const plugin = normalize({cwd, options: {}, logger: t.context.logger}, '', pluginFunction, {});
const plugin = await normalize({cwd, options: {}, logger: t.context.logger}, '', pluginFunction, {});

t.is(plugin.pluginName, '[Function: pluginFunction]');
t.is(typeof plugin, 'function');
});

test('Normalize and load plugin that retuns multiple functions', (t) => {
const plugin = normalize(
test('Normalize and load plugin that retuns multiple functions', async (t) => {
const plugin = await normalize(
{cwd, options: {}, logger: t.context.logger},
'verifyConditions',
'./test/fixtures/multi-plugin',
Expand All @@ -100,7 +100,7 @@ test('Normalize and load plugin that retuns multiple functions', (t) => {

test('Wrap "analyzeCommits" plugin in a function that validate the output of the plugin', async (t) => {
const analyzeCommits = stub().resolves(2);
const plugin = normalize(
const plugin = await normalize(
{cwd, options: {}, stderr: t.context.stderr, logger: t.context.logger},
'analyzeCommits',
analyzeCommits,
Expand All @@ -118,7 +118,7 @@ test('Wrap "analyzeCommits" plugin in a function that validate the output of the

test('Wrap "generateNotes" plugin in a function that validate the output of the plugin', async (t) => {
const generateNotes = stub().resolves(2);
const plugin = normalize(
const plugin = await normalize(
{cwd, options: {}, stderr: t.context.stderr, logger: t.context.logger},
'generateNotes',
generateNotes,
Expand All @@ -136,7 +136,7 @@ test('Wrap "generateNotes" plugin in a function that validate the output of the

test('Wrap "publish" plugin in a function that validate the output of the plugin', async (t) => {
const publish = stub().resolves(2);
const plugin = normalize(
const plugin = await normalize(
{cwd, options: {}, stderr: t.context.stderr, logger: t.context.logger},
'publish',
publish,
Expand All @@ -154,7 +154,7 @@ test('Wrap "publish" plugin in a function that validate the output of the plugin

test('Wrap "addChannel" plugin in a function that validate the output of the plugin', async (t) => {
const addChannel = stub().resolves(2);
const plugin = normalize(
const plugin = await normalize(
{cwd, options: {}, stderr: t.context.stderr, logger: t.context.logger},
'addChannel',
addChannel,
Expand All @@ -174,7 +174,7 @@ test('Plugin is called with "pluginConfig" (with object definition) and input',
const pluginFunction = stub().resolves();
const pluginConf = {path: pluginFunction, conf: 'confValue'};
const options = {global: 'globalValue'};
const plugin = normalize({cwd, options, logger: t.context.logger}, '', pluginConf, {});
const plugin = await normalize({cwd, options, logger: t.context.logger}, '', pluginConf, {});
await plugin({options: {}, param: 'param'});

t.true(
Expand All @@ -189,7 +189,7 @@ test('Plugin is called with "pluginConfig" (with array definition) and input', a
const pluginFunction = stub().resolves();
const pluginConf = [pluginFunction, {conf: 'confValue'}];
const options = {global: 'globalValue'};
const plugin = normalize({cwd, options, logger: t.context.logger}, '', pluginConf, {});
const plugin = await normalize({cwd, options, logger: t.context.logger}, '', pluginConf, {});
await plugin({options: {}, param: 'param'});

t.true(
Expand All @@ -206,7 +206,7 @@ test('Prevent plugins to modify "pluginConfig"', async (t) => {
});
const pluginConf = {path: pluginFunction, conf: {subConf: 'originalConf'}};
const options = {globalConf: {globalSubConf: 'originalGlobalConf'}};
const plugin = normalize({cwd, options, logger: t.context.logger}, '', pluginConf, {});
const plugin = await normalize({cwd, options, logger: t.context.logger}, '', pluginConf, {});
await plugin({options: {}});

t.is(pluginConf.conf.subConf, 'originalConf');
Expand All @@ -218,21 +218,21 @@ test('Prevent plugins to modify its input', async (t) => {
options.param.subParam = 'otherParam';
});
const input = {param: {subParam: 'originalSubParam'}, options: {}};
const plugin = normalize({cwd, options: {}, logger: t.context.logger}, '', pluginFunction, {});
const plugin = await normalize({cwd, options: {}, logger: t.context.logger}, '', pluginFunction, {});
await plugin(input);

t.is(input.param.subParam, 'originalSubParam');
});

test('Return noop if the plugin is not defined', (t) => {
const plugin = normalize({cwd, options: {}, logger: t.context.logger});
test('Return noop if the plugin is not defined', async (t) => {
const plugin = await normalize({cwd, options: {}, logger: t.context.logger});

t.is(plugin, noop);
});

test('Always pass a defined "pluginConfig" for plugin defined with string', async (t) => {
// Call the normalize function with the path of a plugin that returns its config
const plugin = normalize(
const plugin = await normalize(
{cwd, options: {}, logger: t.context.logger},
'',
'./test/fixtures/plugin-result-config',
Expand All @@ -245,7 +245,7 @@ test('Always pass a defined "pluginConfig" for plugin defined with string', asyn

test('Always pass a defined "pluginConfig" for plugin defined with path', async (t) => {
// Call the normalize function with the path of a plugin that returns its config
const plugin = normalize(
const plugin = await normalize(
{cwd, options: {}, logger: t.context.logger},
'',
{path: './test/fixtures/plugin-result-config'},
Expand All @@ -256,8 +256,8 @@ test('Always pass a defined "pluginConfig" for plugin defined with path', async
t.deepEqual(pluginResult.pluginConfig, {});
});

test('Throws an error if the plugin return an object without the expected plugin function', (t) => {
const error = t.throws(() =>
test('Throws an error if the plugin return an object without the expected plugin function', async (t) => {
const error = await t.throwsAsync(() =>
normalize({cwd, options: {}, logger: t.context.logger}, 'inexistantPlugin', './test/fixtures/multi-plugin', {})
);

Expand All @@ -267,8 +267,9 @@ test('Throws an error if the plugin return an object without the expected plugin
t.truthy(error.details);
});

test('Throws an error if the plugin is not found', (t) => {
t.throws(() => normalize({cwd, options: {}, logger: t.context.logger}, 'inexistantPlugin', 'non-existing-path', {}), {
test('Throws an error if the plugin is not found', async (t) => {
await t.throwsAsync(() => normalize({cwd, options: {}, logger: t.context.logger}, 'inexistantPlugin', 'non-existing-path', {}),
{
message: /Cannot find module 'non-existing-path'/,
code: 'MODULE_NOT_FOUND',
instanceOf: Error,
Expand Down