Skip to content

fix(@angular/build): allow library outputPath within project root - #34242

Merged
alan-agius4 merged 1 commit into
angular:mainfrom
alan-agius4:fix/library-output-path-root
Oct 2, 2026
Merged

alan-agius4 merged 1 commit into
angular:mainfrom
alan-agius4:fix/library-output-path-root

Conversation

@alan-agius4

Copy link
Copy Markdown
Collaborator

Remove the upfront check restricting outputPath from being within or containing the project root in the library builder. This is needed for Bazel builds where the output directory is located under the workspace/project root, and matches the behavior of the application builder which relies on deleteOutputDir when cleaning.

Remove the upfront check restricting `outputPath` from being within or
containing the project root in the library builder. This is needed for
Bazel builds where the output directory (`bazel-out/...`) is located
under the workspace/project root, and matches the behavior of the
application builder which relies on `deleteOutputDir` when cleaning.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request removes the validation check that prevents the outputPath from being equal to, containing, or being located within the projectRoot in the library builder options. A review comment points out that removing this entire validation block could lead to accidental deletion of the project root or workspace (since deleteOutputPath defaults to true), and suggests keeping the safeguards that prevent the output path from being equal to or a parent of the project root.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/angular/build/src/builders/library/options.ts (37-48)

high

Removing the entire validation block allows the outputPath to be equal to the projectRoot or to contain the projectRoot (i.e., being a parent directory). Since deleteOutputPath defaults to true, running the builder in these configurations could result in the accidental deletion of the entire project root or workspace, leading to severe data loss.

To safely allow the outputPath to be located within the project root (as intended), we should only remove the isSubDirectory(resolvedOutputPath, projectRoot) check, while keeping the safeguards that prevent the output path from being equal to or a parent of the project root.

  const outputPath = options.outputPath ?? path.join(workspaceRoot, 'dist', projectName);
  const resolvedOutputPath = path.resolve(workspaceRoot, outputPath);
  if (
    resolvedOutputPath === projectRoot ||
    isSubDirectory(projectRoot, resolvedOutputPath)
  ) {
    throw new Error(
      `The 'outputPath' (${resolvedOutputPath}) cannot be the project root or contain the project root.`,
    );
  }

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Oct 2, 2026
@alan-agius4
alan-agius4 requested a review from clydin October 2, 2026 08:32
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 2, 2026

@clydin clydin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the output is in the project root, I think .d.ts files and assets from the built output could potentially end up being picked up by the build. Default generated project shouldn't exhibit this but customizations could unintentionally cause it.

Comment on lines -37 to -49
const outputPath = options.outputPath ?? path.join(workspaceRoot, 'dist', projectName);
const resolvedOutputPath = path.resolve(workspaceRoot, outputPath);
if (
resolvedOutputPath === projectRoot ||
isSubDirectory(resolvedOutputPath, projectRoot) ||
isSubDirectory(projectRoot, resolvedOutputPath)
) {
throw new Error(
`The 'outputPath' (${resolvedOutputPath}) cannot be the project root, ` +
`contain the project root, or be located within the project root.`,
);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably should keep this safety check but remove isSubDirectory(projectRoot, resolvedOutputPath) and update the error message to cannot be the project root or contain the project root.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That would break the usage in case it’s a single project repo, and the dist is in the project root.

I can add the check that is not the project root in a follow up, so that this check is also added in the application builder.

@clydin clydin Oct 2, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dist in the project root would be fine. Its the case where the project root is inside the output path itself that is problematic since that would delete the entire project root on a build.

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 2, 2026
@alan-agius4
alan-agius4 merged commit 4244eb7 into angular:main Oct 2, 2026
44 checks passed
@alan-agius4

Copy link
Copy Markdown
Collaborator Author

This PR was merged into the repository. The changes were merged into the following branches:

@alan-agius4
alan-agius4 deleted the fix/library-output-path-root branch October 2, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @angular/build target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants