Skip to content
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

Add support for declaring specific endpoints to be built #120

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

con322
Copy link

@con322 con322 commented Jul 10, 2024

Description

This PR adds the ability to define the endpoints you want to build so that if your working on a specific part of a project you can optimise performance/speed of builds for what your working on via defining the endpoints after @ symbol. For example for all projects you can use @frontend or if your working with multiple project and only want to build the frontend assets for one you can use test-plugin@frontend. Fixes #46

Change log

  • Add logic so that projects positional can have entrypoints defined after them by appened @ along with the entrypoint ie test-plugin@frontend or multiple via + such as test-plugin@frontend+editor.
  • Adds new property to project config called filteredEntrypoints which will contain the specified entrypoints if set, defaults to empty array.
  • Add cli test.
  • Add unit test for new util function (getFilteredEntryPoints).

Code reviewer(s)

  • Is filteredEntrypoints the right variable name?
  • Should some kind of validation be performed to ensure projects/filtered endpoints are following the correct format (plugin@fronted+editor, @frontend, etc)?
  • README.md will be updated once an initial review has been done and the above questions have been answered as things are subject to change.

Testing

Example Individual Projects build command: build-tools build my-plugin@frontend.
Example Site-wide build command: build-tools build @frontend.

  • Verify Individual Projects build command works without @ specified
  • Verify Individual Projects build command works with single @frontend specified
  • Verify Individual Projects build command works with multiple @frontend+editor specified
  • Verify Site-wide build command works without @ specified
  • Verify Site-wide build command works with single @frontend specified
  • Verify Site-wide build command works with multiple @frontend+editor specified

@con322 con322 changed the title Feature/targeted entrypoints Add support for declaring specific endpoints to be built Jul 10, 2024
@con322 con322 marked this pull request as ready for review July 11, 2024 14:33
@con322 con322 requested a review from ampersarnie as a code owner July 11, 2024 14:33
jaymcp
jaymcp previously approved these changes Jul 12, 2024
Copy link
Member

@jaymcp jaymcp left a comment

Choose a reason for hiding this comment

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

Thanks Connor! Works for me 😊

Copy link
Member

@ampersarnie ampersarnie left a comment

Choose a reason for hiding this comment

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

I've not tested yet and only reviewed code. Looks great, I've left some minor feedback for you. Also, can you update the docs in docs/ with how this works to provide guidance to users.

src/utils/get-filtered-entrypoints.js Show resolved Hide resolved
src/utils/entrypoints.js Outdated Show resolved Hide resolved
Copy link
Member

@ampersarnie ampersarnie left a comment

Choose a reason for hiding this comment

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

Good to go after these are removed.

Copy link
Member

Choose a reason for hiding this comment

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

Just spotted this, yarn isn't used in the project so can this be removed to avoid confusion.

Copy link
Author

Choose a reason for hiding this comment

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

Removed this :)

yarn.lock Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Just spotted this, yarn isn't used in the project so can this be removed to avoid confusion.

Copy link
Author

Choose a reason for hiding this comment

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

Removed this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Watch/build a specific entry point
3 participants