-
Notifications
You must be signed in to change notification settings - Fork 945
feat: add support for allowScripts setting #10007
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for the allowScripts configuration setting to provide fine-grained control over dependency lifecycle scripts (preinstall, install, postinstall). The setting allows explicitly allowing, blocking, or warning about specific package scripts, complementing the existing neverBuiltDependencies option.
- Adds
allowScriptsconfiguration option with support for boolean and 'warn' values - Updates workspace JSON schema with comprehensive documentation and examples
- Integrates the setting through all layers: config → installer → package manager → pnpm
- Includes E2E test coverage for basic allow/block functionality
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| workspace.jsonc | Updates @pnpm/default-reporter to version 1002.1.0 for new functionality support |
| workspace-jsonc-schema.json | Adds JSON schema definition for allowScripts with oneOf boolean/'warn' validation and examples |
| scopes/workspace/install/install.main.runtime.ts | Passes allowScripts config from dependency resolver to install options |
| scopes/dependencies/pnpm/pnpm.package-manager.ts | Forwards allowScripts from install options to pnpm implementation |
| scopes/dependencies/pnpm/lynx.ts | Implements resolveScriptPolicies function to convert allowScripts to pnpm's script control options and adds instructional text for reporter |
| scopes/dependencies/pnpm/lockfile-deps-graph-converter.spec.ts | Converts imports to type imports for PackagesMap and DependencyEdge |
| scopes/dependencies/dependency-resolver/package-manager.ts | Adds allowScripts to PackageManagerInstallOptions type definition |
| scopes/dependencies/dependency-resolver/dependency-resolver.main.runtime.ts | Adds allowScripts parameter to DependencyInstaller constructor |
| scopes/dependencies/dependency-resolver/dependency-resolver-workspace-config.ts | Documents allowScripts config option in workspace config interface |
| scopes/dependencies/dependency-resolver/dependency-installer.ts | Adds allowScripts property and passes it to package manager, removes hardcoded 'core-js' handling |
| e2e/harmony/dependencies/allow-scripts.e2e.ts | Adds E2E test verifying allow/block behavior for dependency scripts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scopes/dependencies/dependency-resolver/dependency-resolver-workspace-config.ts
Outdated
Show resolved
Hide resolved
…rkspace-config.ts Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed Changes