Skip to content

Conversation

realtimetodie
Copy link
Contributor

@realtimetodie realtimetodie commented Dec 8, 2024

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Currently, when you create a custom builder for Angular CLI, the builder schema can only be a reference to a local file in the manifest directory. This is cumbersome, for example when you want to reference an external schema from a module instead.

This is now working:

"schema": "./src/builders/browser-esbuild/schema.json"

This is not working:

"schema": "@angular-devkit/build-angular/src/builders/browser-esbuild/schema.json"

What is the new behavior?

It is now possible to require a builder schema from a module.

This is now working:

"schema": "./src/builders/browser-esbuild/schema.json"

and this too:

"schema": "@angular-devkit/build-angular/src/builders/browser-esbuild/schema.json"

Internal mechanics:

The localRequire.resolve function will look up the location of a module, but rather than loading the module, just return the resolved filename:

schemaPath = localRequire.resolve(schemaPath, {
  paths: [basePath],
});

If the module can not be found, a MODULE_NOT_FOUND error is thrown by Node.js and the filepath will be composed using the local manifest directory alternatively.

Error message:

In any case, the schema will be read using readFileSync and the error message will always be the same:

ENOENT: no such file or directory: <path>

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This comment was marked as resolved.

@realtimetodie realtimetodie force-pushed the feat-require-schema branch 2 times, most recently from 8126024 to 2d46bd1 Compare December 8, 2024 14:59
@alan-agius4 alan-agius4 requested a review from clydin December 10, 2024 15:42
@realtimetodie
Copy link
Contributor Author

@alan-agius4 I fixed the linting issue, please run the workflow again

@jkrems
Copy link
Contributor

jkrems commented Dec 10, 2024

@realtimetodie It looks like the lint action now fails because of the commit message. Can you try amending the first commit with the lint fixes (e.g. using git rebase -i and fixup)?

@realtimetodie
Copy link
Contributor Author

@jkrems done, sorry for the inconvenience

@angular-robot angular-robot bot added the area: docs Related to the documentation label Dec 11, 2024
@realtimetodie realtimetodie requested a review from clydin December 11, 2024 21:17
Copy link
Member

@clydin clydin left a comment

Choose a reason for hiding this comment

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

Can you squash the commits? Otherwise this looks good.

Thank you for the contribution.

@angular-robot angular-robot bot removed the area: docs Related to the documentation label Dec 11, 2024
@realtimetodie
Copy link
Contributor Author

I squashed the commits. Thank your for taking the time to review my contribution.

@clydin clydin added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Dec 12, 2024
@jkrems jkrems merged commit 2b8a02b into angular:main Dec 12, 2024
32 checks passed
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: @angular-devkit/architect detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

local schema
3 participants