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.

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 plugin system #304

Merged
merged 7 commits into from
Jun 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion config/tslint/tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
},
{ "type": "type", "format": "PascalCase" },
{ "type": "genericTypeParameter", "suffix": "T" },
{ "type": "enumMember", "format": "PascalCase" }
{ "type": "enum", "format": "PascalCase" },
{ "type": "enumMember", "format": "UPPER_CASE" }
]
}
}
2 changes: 2 additions & 0 deletions packages/buidler-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ The recommended way of using Buidler is through a local installation in your pro
Be careful about inconsistent behavior across different projects that use different Buidler versions.

npm -g install @nomiclabs/buidler

If you choose to install Buidler globally, you have to do the same for its plugins and their dependencies.

## Documentation

Expand Down
30 changes: 30 additions & 0 deletions packages/buidler-core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/buidler-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"fs-extra": "^7.0.1",
"glob": "^7.1.3",
"io-ts": "^1.8.6",
"is-installed-globally": "^0.2.0",
"lodash": "^4.17.11",
"mocha": "^5.2.0",
"semver": "^5.6.0",
Expand Down
6 changes: 5 additions & 1 deletion packages/buidler-core/src/internal/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class BuidlerContext {
public readonly tasksDSL = new TasksDSL();
public readonly extendersManager = new ExtenderManager();
public environment?: BuidlerRuntimeEnvironment;
public configPath?: string;
public readonly loadedPlugins: string[] = [];

public setBuidlerRuntimeEnvironment(env: BuidlerRuntimeEnvironment) {
if (this.environment !== undefined) {
Expand All @@ -53,4 +53,8 @@ export class BuidlerContext {
}
return this.environment;
}

public setPluginAsLoaded(pluginName: string) {
this.loadedPlugins.push(pluginName);
}
}
10 changes: 9 additions & 1 deletion packages/buidler-core/src/internal/core/config/config-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from "../../../types";
import { BuidlerContext } from "../../context";
import * as argumentTypes from "../params/argumentTypes";
import { usePlugin as usePluginImplementation } from "../plugins";

export function task<ArgsT extends TaskArguments>(
name: string,
Expand Down Expand Up @@ -82,4 +83,11 @@ export function extendEnvironment(extender: EnvironmentExtender) {
extenderManager.add(extender);
}

export { usePlugin } from "../plugins";
/**
* Loads a Buidler plugin
* @param pluginName The plugin name.
*/
export function usePlugin(pluginName: string) {
const ctx = BuidlerContext.getBuidlerContext();
usePluginImplementation(ctx, pluginName);
}
15 changes: 0 additions & 15 deletions packages/buidler-core/src/internal/core/config/config-loading.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,6 @@ export function loadConfigAndTasks(configPath?: string): ResolvedBuidlerConfig {
([key, value]) => (globalAsAny[key] = value)
);

// This is a horrible hack that deserves an explanation.
// - config files can execute the usePlugin function, which is imported
// from config.ts.
// - There's no way to pass it arguments.
// - Internally, usePlugin calls require, which should use the same
// node_module's paths than the Buidler project.
// - Except that it doesn't when we are linking Buidler for local tests.
// - node resolves symlinks before loading modules, so imports from
// Buidler files are run in this context, not inside the Buidler project.
// - We solve this by using require.resolve and specifying the paths,
// but we need the config path in order to do so.
// - We set the config path into the BuidlerContext and cry a little 😢
const ctx = BuidlerContext.getBuidlerContext();
ctx.configPath = configPath;

loadPluginFile(__dirname + "/../tasks/builtin-tasks");

const defaultConfig = importCsjOrEsModule("./default-config");
Expand Down
8 changes: 4 additions & 4 deletions packages/buidler-core/src/internal/core/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,14 @@ To learn more about Buidler's configuration, please go to https://buidler.dev/do
MISSING_DEPENDENCY: {
number: 801,
message:
"Plugin %s requires %s to be installed.\n" +
"Please run: npm install --save-dev %s@%s"
"Plugin %s requires %s to be installed.\n%s" +
"Please run: npm install --save-dev%s %s@%s"
},
DEPENDENCY_VERSION_MISMATCH: {
number: 802,
message:
"Plugin %s requires %s version %s but got %s.\n" +
"If you haven't installed %s manually, please run: npm install --save-dev %s@%s\n" +
"Plugin %s requires %s version %s but got %s.\n%s" +
"If you haven't installed %s manually, please run: npm install --save-dev%s %s@%s\n" +
"If you have installed %s yourself, please reinstall it with a valid version."
},
OLD_STYLE_IMPORT_DETECTED: {
Expand Down
34 changes: 34 additions & 0 deletions packages/buidler-core/src/internal/core/execution-mode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* This module defines different Buidler execution modes and autodetects them.
*
* IMPORTANT: This will have to be revisited once Yarn PnP and npm's tink get
* widely adopted.
*/
export enum ExecutionMode {
EXECUTION_MODE_TS_NODE_TESTS,
EXECUTION_MODE_LINKED,
EXECUTION_MODE_GLOBAL_INSTALLATION,
EXECUTION_MODE_LOCAL_INSTALLATION
}

const workingDirectoryOnLoad = process.cwd();

export function getExecutionMode(): ExecutionMode {
const isInstalled = __filename.includes("node_modules");

if (!isInstalled) {
// When running the tests with ts-node we set the CWD to the root of
// buidler-core. We could check if the __filename ends with .ts
if (__dirname.startsWith(workingDirectoryOnLoad)) {
return ExecutionMode.EXECUTION_MODE_TS_NODE_TESTS;
}

return ExecutionMode.EXECUTION_MODE_LINKED;
}

if (require("is-installed-globally")) {
return ExecutionMode.EXECUTION_MODE_GLOBAL_INSTALLATION;
}

return ExecutionMode.EXECUTION_MODE_LOCAL_INSTALLATION;
}
84 changes: 70 additions & 14 deletions packages/buidler-core/src/internal/core/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,82 @@ import { BuidlerContext } from "../context";
import { getClosestCallerPackage } from "../util/caller-package";

import { BuidlerError, ERRORS } from "./errors";
import { ExecutionMode, getExecutionMode } from "./execution-mode";

interface PackageJson {
name: string;
version: string;
peerDependencies: {
[name: string]: string;
};
}

export function usePlugin(pluginName: string) {
const ctx = BuidlerContext.getBuidlerContext();
const configFileDir = path.dirname(ctx.configPath!);
/**
* Validates a plugin dependencies and loads it.
* @param pluginName - The plugin name
* @param buidlerContext - The BuidlerContext
* @param from - Where to resolve plugins and dependencies from. Only for
* testing purposes.
*/
export function usePlugin(
buidlerContext: BuidlerContext,
pluginName: string,
from?: string
) {
// We have a special case for `ExecutionMode.EXECUTION_MODE_LINKED`
//
// If Buidler is linked, a require without `from` would be executed in the
// context of Buidler, and not find any plugin (linked or not). We workaround
// this by using the CWD here.
//
// This is not ideal, but the only reason to link Buidler is testing.
if (
from === undefined &&
getExecutionMode() === ExecutionMode.EXECUTION_MODE_LINKED
) {
from = process.cwd();
}

const pluginPackageJson = readPackageJson(pluginName, configFileDir);
const pluginPackageJson = readPackageJson(pluginName, from);

if (pluginPackageJson === undefined) {
throw new BuidlerError(ERRORS.PLUGINS.NOT_INSTALLED, pluginName);
}

// We use the package.json's version of the name, as it is normalized.
pluginName = pluginPackageJson.name;

if (buidlerContext.loadedPlugins.includes(pluginName)) {
return;
}

let globalFlag = "";
let globalWarning = "";
if (getExecutionMode() === ExecutionMode.EXECUTION_MODE_GLOBAL_INSTALLATION) {
globalFlag = " --global";
globalWarning =
"You are using a global installation of Buidler. Plugins and their dependencies must also be global.\n";
}

if (pluginPackageJson.peerDependencies !== undefined) {
for (const [dependencyName, versionSpec] of Object.entries(
pluginPackageJson.peerDependencies
)) {
const dependencyPackageJson = readPackageJson(
dependencyName,
configFileDir
);
const dependencyPackageJson = readPackageJson(dependencyName, from);

let installExtraFlags = globalFlag;

if (versionSpec.match(/^[0-9]/) !== null) {
installExtraFlags += " --save-exact";
}

if (dependencyPackageJson === undefined) {
throw new BuidlerError(
ERRORS.PLUGINS.MISSING_DEPENDENCY,
pluginName,
dependencyName,
globalWarning,
installExtraFlags,
dependencyName,
versionSpec
);
Expand All @@ -51,7 +95,9 @@ export function usePlugin(pluginName: string) {
dependencyName,
versionSpec,
installedVersion,
globalWarning,
dependencyName,
installExtraFlags,
dependencyName,
versionSpec,
dependencyName
Expand All @@ -60,8 +106,11 @@ export function usePlugin(pluginName: string) {
}
}

const pluginPath = require.resolve(pluginName, { paths: [configFileDir] });
const options = from !== undefined ? { paths: [from] } : undefined;
const pluginPath = require.resolve(pluginName, options);
loadPluginFile(pluginPath);

buidlerContext.setPluginAsLoaded(pluginName);
}

export function loadPluginFile(absolutePluginFilePath: string) {
Expand All @@ -74,14 +123,13 @@ export function loadPluginFile(absolutePluginFilePath: string) {

export function readPackageJson(
packageName: string,
from: string
from?: string
): PackageJson | undefined {
try {
const options = from !== undefined ? { paths: [from] } : undefined;
const packageJsonPath = require.resolve(
path.join(packageName, "package.json"),
{
paths: [from]
}
options
);

return require(packageJsonPath);
Expand All @@ -102,8 +150,16 @@ export function ensurePluginLoadedWithUsePlugin() {

for (const callSite of stack) {
const fileName = callSite.getFileName();
if (fileName === null) {
continue;
}

const functionName = callSite.getFunctionName();
if (fileName === __filename && functionName === loadPluginFile.name) {

if (
path.basename(fileName) === path.basename(__filename) &&
functionName === loadPluginFile.name
) {
return;
}
}
Expand Down
16 changes: 7 additions & 9 deletions packages/buidler-core/src/internal/core/typescript-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,9 @@ import colors from "ansi-colors";
import * as fs from "fs";
import * as path from "path";

const NODE_MODULES_DIR = "node_modules";
import { ExecutionMode, getExecutionMode } from "./execution-mode";

/**
* This function returns true only if Buidler was installed as a dependency. It
* returns false otherwise, including when it's being linked.
*/
function isBuidlerInstalledAsADependency() {
return __dirname.lastIndexOf(NODE_MODULES_DIR) !== -1;
}
const NODE_MODULES_DIR = "node_modules";

function getBuidlerNodeModules() {
return __dirname.substring(
Expand All @@ -23,7 +17,11 @@ let cachedIsTypescriptSupported: boolean | undefined;

export function isTypescriptSupported() {
if (cachedIsTypescriptSupported === undefined) {
if (isBuidlerInstalledAsADependency()) {
const executionMode = getExecutionMode();
if (
executionMode === ExecutionMode.EXECUTION_MODE_LOCAL_INSTALLATION ||
executionMode === ExecutionMode.EXECUTION_MODE_GLOBAL_INSTALLATION
) {
const nodeModules = getBuidlerNodeModules();
cachedIsTypescriptSupported =
fs.existsSync(path.join(nodeModules, "typescript")) &&
Expand Down
4 changes: 2 additions & 2 deletions packages/buidler-core/src/internal/util/scripts-runner.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { BuidlerArguments } from "../../types";
import { ExecutionMode, getExecutionMode } from "../core/execution-mode";
import { getEnvVariablesMap } from "../core/params/env-variables";

export async function runScript(
Expand Down Expand Up @@ -46,8 +47,7 @@ export async function runScriptWithBuidler(
}

function getTsNodeArgsIfNeeded() {
// This means we are running the tests
if (!__filename.endsWith(".ts")) {
if (getExecutionMode() !== ExecutionMode.EXECUTION_MODE_TS_NODE_TESTS) {
return [];
}

Expand Down
Loading