Skip to content

Commit

Permalink
Improve config resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Feb 13, 2022
1 parent 456e445 commit 0500687
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 64 deletions.
4 changes: 2 additions & 2 deletions src/bin/config/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ export const ALL_CONFIG = {
This can be:
- A file path
- A Node module exporting a configuration file.
The module name must start with "spyd-config-".
- A Node module name starting with "spyd-config-", optionally prefixed with
any npm "@scope/"
By default, any "spyd.*" file in the current or parent directories is used.
It can also be inside a "benchmark" or "packages/spyd-config-*" sub-directory.
Expand Down
24 changes: 9 additions & 15 deletions src/config/load/npx.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { env } from 'process'

import { CONFIG_NPM_PREFIX } from './resolvers.js'
import { isConfigFileModule } from './resolvers.js'

// In principle, users can use `npx` with the "npm" resolver by doing:
// npx --package=spyd-config-{name} spyd --config=spyd-config-{name} ...
// As a convenience, we allow this shortcut syntax:
// npx --package=spyd-config-{name} spyd ...
// We do this by:
// - Detecting whether `npx --package=spyd-config-{name}` is used
// - This detection is based on `npm_*` environment variables injected by
// `npx`
// - Adding it as `--config=spyd-config-{name}`
// This behaves as if `--config=spyd-config-{name}` has been specified:
// npx --package={spydConfig} spyd --config={spydConfig} ...
// Where {spydConfig} is [@scope/]spyd-config-{name}
// As a convenience, we allow skipping the `--config` flag. We do this by:
// - Detecting whether `npx --package={spydConfig}` is used
// - Based on `npm_*` environment variables injected by `npx`
// - Adding it as `--config={spydConfig}`
// This behaves as if `--config={spydConfig}` had been specified:
// - Additional `--config` flags are kept
// - The `--config` flag does not use its default value
export const addNpxShortcut = function (configFlags) {
Expand Down Expand Up @@ -41,9 +39,5 @@ const isNpxCall = function () {
}

const getNpxConfigs = function () {
return env.npm_config_package.split('\n').filter(isSharedConfig)
}

const isSharedConfig = function (npxPackage) {
return npxPackage.startsWith(`${CONFIG_NPM_PREFIX}-`)
return env.npm_config_package.split('\n').filter(isConfigFileModule)
}
54 changes: 24 additions & 30 deletions src/config/load/resolvers.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,41 @@
import { createRequire } from 'module'

import { wrapError } from '../../error/wrap.js'
import { resolveModuleName } from '../module.js'

// The `config` can be:
// - a file path
// - a Node module name starting with "spyd-config-"
// - a Node module name starting with "[@scope/]spyd-config-"
// - a "resolver:arg" string which applies resolver-specific logic
export const isConfigFilePath = function (configOpt) {
return !isNpmResolver(configOpt) && !isResolver(configOpt)
}

export const useResolvers = async function (configOpt, base) {
if (isNpmResolver(configOpt)) {
return resolveNpm(configOpt, base)
// - a file path
export const useResolvers = async function (configOpt, { cwd }) {
if (isConfigFileModule(configOpt)) {
return resolveNpm(configOpt, cwd)
}

if (isResolver(configOpt)) {
return await useResolver(configOpt, base)
if (isConfigFileResolver(configOpt)) {
return await useResolver(configOpt, cwd)
}

return configOpt
}

// Configs can be Node modules.
// They must be named "spyd-config-{name}" to enforce naming convention and
// allow distinguishing them from file paths.
// We do not use a shorter id like "npm:{name}" so users do not need two
// different ids: one for `npm install` and one for the `config` property.
const isNpmResolver = function (configOpt) {
return configOpt.startsWith(`${CONFIG_NPM_PREFIX}-`)
export const isConfigFileModule = function (configOpt) {
return configOpt.includes(CONFIG_NPM_PREFIX)
}

export const CONFIG_NPM_PREFIX = 'spyd-config'
export const CONFIG_NPM_PREFIX = 'spyd-config-'

// TODO: use import.meta.resolve() when available
const resolveNpm = function (configOpt, base) {
const baseUrl = new URL(base, import.meta.url)
const isConfigFileResolver = function (configOpt) {
return RESOLVER_REGEXP.test(configOpt)
}

export const isConfigFilePath = function (configOpt) {
return !isConfigFileModule(configOpt) && !isConfigFileResolver(configOpt)
}

// TODO: use import.meta.resolve() when available
const resolveNpm = function (configOpt, cwd) {
try {
return resolveModuleName(configOpt, CONFIG_NPM_PREFIX, baseUrl)
return createRequire(`${cwd}/`).resolve(configOpt)
} catch (error) {
throw wrapError(
error,
Expand All @@ -52,11 +50,7 @@ This Node module was not found, please ensure it is installed.\n`,
// Their name must be namespaced with "{resolver}:".
// We do not use this type of namespaces for the other resolvers since they are
// more commonly used.
const isResolver = function (configOpt) {
return RESOLVER_REGEXP.test(configOpt)
}

const useResolver = async function (configOpt, base) {
const useResolver = async function (configOpt, cwd) {
const {
groups: { name, arg },
} = RESOLVER_REGEXP.exec(configOpt)
Expand All @@ -68,7 +62,7 @@ const useResolver = async function (configOpt, base) {
throw new Error(`must use an existing resolver among ${resolvers}`)
}

return await resolverFunc(arg, base)
return await resolverFunc(arg, cwd)
}

// We require at least two characters to differentiate from absolute file paths
Expand Down
32 changes: 27 additions & 5 deletions src/config/normalize/lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { resolve, basename } from 'path'

import fastGlob from 'fast-glob'
import { isNotJunk } from 'junk'
import { pathExists } from 'path-exists'
import { isDirectory } from 'path-type'

import { wrapError } from '../../../error/wrap.js'

import { callValueFunc, callUserFunc } from './call.js'

Expand All @@ -10,8 +14,9 @@ import { callValueFunc, callUserFunc } from './call.js'
// to ensure it is evaluated at runtime.
export const addCwd = async function ({ cwd = DEFAULT_CWD, opts }) {
const cwdA = await callUserFunc(cwd, opts)
await validatePath(cwdA, opts)
return { ...opts, cwd: cwdA }
await callValueFunc(checkCwd, cwdA, opts)
const cwdB = resolve(cwdA)
return { ...opts, cwd: cwdB }
}

const DEFAULT_CWD = '.'
Expand All @@ -29,13 +34,30 @@ export const resolvePath = async function (
return value
}

await validatePath(value, opts)
await callValueFunc(checkPath, value, opts)

return glob ? await resolveGlob(cwd, value) : resolve(cwd, value)
}

export const validatePath = async function (value, opts) {
await callValueFunc(checkPath, value, opts)
const checkCwd = async function (value) {
try {
await checkDir(value)
} catch (error) {
// Errors in `cwd` are not user errors, i.e. should not start with `must`
throw wrapError(error, "'s current directory")
}
}

const checkDir = async function (value) {
checkPath(value)

if (!(await pathExists(value))) {
throw new Error('must be an existing file.')
}

if (!(await isDirectory(value))) {
throw new Error('must be a directory.')
}
}

const checkPath = function (value) {
Expand Down
2 changes: 1 addition & 1 deletion src/config/normalize/lib/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const quotePropName = function (propName) {
propName.slice(0, lastSpaceIndex + 1),
propName.slice(lastSpaceIndex + 1),
]
return `${firstWords}"${lastWord}" `
return `${firstWords}"${lastWord}"`
}

const callPrefix = async function ({ prefix, ...opts }) {
Expand Down
3 changes: 2 additions & 1 deletion src/config/plugin/lib/id.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { isAbsolute } from 'path'

import { wrapError } from '../../../error/wrap.js'
import { resolveModuleName } from '../../module.js'

import { resolveModuleName } from './module.js'

// `pluginConfig.id` can be:
// - The direct value
Expand Down
4 changes: 2 additions & 2 deletions src/config/module.js → src/config/plugin/lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { createRequire } from 'module'
// Resolve Node module ids to absolute file paths.
// We enforce a npm package naming convention for all plugins.
// TODO: use import.meta.resolve() when available
export const resolveModuleName = function (id, modulePrefix, base) {
export const resolveModuleName = function (id, modulePrefix, cwd) {
const moduleName = getModuleName(id, modulePrefix)
return createRequire(base).resolve(moduleName)
return createRequire(`${cwd}/`).resolve(moduleName)
}

// We allow module names to be either:
Expand Down
22 changes: 14 additions & 8 deletions src/error/wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import omit from 'omit.js'
import { normalizeError } from './utils.js'

// Wrap a child error with a new message and type
export const wrapError = function (error, message, ErrorType) {
export const wrapError = function (error, prefix, ErrorType) {
const errorA = normalizeError(error)
const errorB = changeErrorType(errorA, ErrorType)
const errorC = wrapErrorMessage(errorB, message)
const errorC = wrapErrorMessage(errorB, prefix)
const errorD = copyStaticProps(errorC, error)
return errorD
}
Expand All @@ -31,19 +31,25 @@ const changeErrorType = function (error, ErrorType) {
}

// Prepend|append a prefix|suffix to an error message
const wrapErrorMessage = function (error, message) {
if (message === '') {
const wrapErrorMessage = function (error, prefix) {
if (prefix === '') {
return error
}

const space = WHITESPACE_END_REGEXP.test(message) ? '' : ' '
error.message = message.startsWith('\n')
? `${error.message}${message}`
: `${message}${space}${error.message.trim()}`
const { message } = error
error.message = prefix.startsWith('\n')
? `${message}${prefix}`
: `${prefix}${getSpaceDelimiter(message, prefix)}${message.trim()}`
fixErrorStack(error)
return error
}

const getSpaceDelimiter = function (message, prefix) {
return WHITESPACE_END_REGEXP.test(prefix) || message.startsWith("'")
? ''
: ' '
}

const WHITESPACE_END_REGEXP = /\s$/u

// `error.name` and `error.message` are prepended to `error.stack`.
Expand Down

0 comments on commit 0500687

Please sign in to comment.