feat: onlyBuiltDependenciesFile #7167
Conversation
| @@ -44,6 +47,10 @@ export function getOptionsFromRootManifest (manifest: ProjectManifest): { | |||
| // @ts-expect-error | |||
| settings['onlyBuiltDependencies'] = onlyBuiltDependencies | |||
| } | |||
| if (onlyBuiltDependenciesFile) { | |||
| // @ts-expect-error | |||
There was a problem hiding this comment.
nitpick: Extracting the interface returned from getOptionsFromRootManifest would be a bit more TypeScript idiomatic. It also allows the // @ts-expect-error to be removed, which can unexpectedly hide other compile errors unrelated to indexing that happen in the future.
export interface OptionsFromRootManifest {
allowedDeprecatedVersions?: AllowedDeprecatedVersions
allowNonAppliedPatches?: boolean
overrides?: Record<string, string>
neverBuiltDependencies?: string[]
onlyBuiltDependencies?: string[]
onlyBuiltDependenciesFile?: string
packageExtensions?: Record<string, PackageExtension>
patchedDependencies?: Record<string, string>
peerDependencyRules?: PeerDependencyRules
}
export function getOptionsFromRootManifest (
manifestDir: string,
manifest: ProjectManifest
): OptionsFromRootManifest { /* ... */ }And then further down:
const settings: OptionsFromRootManifest = {
allowedDeprecatedVersions,
allowNonAppliedPatches,
overrides,
neverBuiltDependencies,
packageExtensions,
peerDependencyRules,
patchedDependencies,
}
if (onlyBuiltDependencies) {
settings['onlyBuiltDependencies'] = onlyBuiltDependencies
}
if (onlyBuiltDependenciesFile) {
settings['onlyBuiltDependenciesFile'] = path.join(manifestDir, onlyBuiltDependenciesFile)
}| @@ -53,6 +54,7 @@ export async function buildModules ( | |||
| warn, | |||
| } | |||
| const chunks = buildSequence(depGraph, rootDepPaths) | |||
| const allowBuild = opts.allowBuild ?? (() => true) | |||
There was a problem hiding this comment.
Nice. I like the abstraction of allowBuild to avoid propagating the concept of onlyBuiltDependenciesFile down to this lower level component.
| @@ -1004,7 +1004,8 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { | |||
| } = await resolveDependencies( | |||
| projects, | |||
| { | |||
| allowBuild: createAllowBuildFunction(opts), | |||
| // In the next major allow build should be just () => true here always | |||
| allowBuild: opts.onlyBuiltDependenciesFile ? () => true : createAllowBuildFunction({ onlyBuiltDependencies: opts.onlyBuiltDependencies, neverBuiltDependencies: opts.neverBuiltDependencies }), | |||
There was a problem hiding this comment.
I might be missing something. If opts.onlyBuiltDependenciesFile is non-null, we'll want to call createAllowBuildFunction, is that right? It currently evaluates to () => true instead.
Or is this intentional and the usage of opts.onlyBuiltDependenciesFile opts into the default behavior in the next major version, which would be to always return () => true?
There was a problem hiding this comment.
Or is this intentional and the usage of opts.onlyBuiltDependenciesFile opts into the default behavior in the next major version, which would be to always return () => true?
yes, correct
* feat: specifying allowed build list via a json file * bump ref pnpm/pnpm#7167
close #7137
TODO: