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 better broccoli-side-watch package #2141

Merged
merged 9 commits into from
Oct 8, 2024
Merged

Conversation

simonihmig
Copy link
Collaborator

@simonihmig simonihmig commented Oct 8, 2024

We have the broccoli-side-watch on the main branch only, as such there are only 0.0.2-unstable.xxx versions released. That version is only able to watch for explicitly passed folders, which is inconvenient when you want to watch for other packages (addons) in a monorepo, as you need to hard-code (or resolve programatically) the actual folders on disk (/path/to/addon/dist), but what you rather want is an easy way to just list the package names, similar to what ember-auto-import's watchDependencies option does.

So this PR is

  • cherry-picking the commit by @balinterdi adding that package to bring it into the stable branch
  • adding support to list (absolute or relative) folders as well as package names
  • adding the watch-utils (and tests) that have been added to eai (Optimize watched directories ember-auto-import#623) to shared-internals, which allows us to get the optimal watched directory given a package name, taking package.json#exports into account (e.g. watching only ./dist for common v2 addons, instead of the whole package folder including ./src). The plan would be to remove these utils from eai and import frrm shared-internals once released.
  • adding broccoli tests
  • converting to TS
  • bumping the package version to 1.0.0 manually (@mansona is this how we tell release-plan to publish a specific stable version?)

@simonihmig simonihmig marked this pull request as ready for review October 8, 2024 10:50
@simonihmig simonihmig requested a review from a team October 8, 2024 10:51

## Usage

Let's assume you have a v2 addon in the `grand-prix` folder of your top-level folder of your Ember app.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a bit confusing to me does it intend to say the below?

Suggested change
Let's assume you have a v2 addon in the `grand-prix` folder of your top-level folder of your Ember app.
Let's assume you have a v2 addon in the `grand-prix` folder of your monorepo that also contains your Ember app.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, probably. I didn't change this, this was coming from the main branch: https://github.com/embroider-build/embroider/blob/main/packages/broccoli-side-watch/README.md. But we can tweak this for sure!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@void-mAlex updated the Readme, if you want to give it another look!?


const app = new EmberApp(defaults, {
trees: {
app: sideWatch('app', { watching: ['./grand-prix/src'] }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't this imply nesting two package.json?
one for the Ember app and inside of it the v2 addon?
that seems wrong to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same here! But yeah, I see what you are meaning. Will go over the Readme once more!

packages/shared-internals/package.json Show resolved Hide resolved
@ef4
Copy link
Contributor

ef4 commented Oct 8, 2024

This looks ready to go. The beta test failure should be fixed on stable now if you merge or rebase.

@simonihmig simonihmig force-pushed the side-watch-packages branch from 0096f79 to ff8ad4c Compare October 8, 2024 16:00
@simonihmig simonihmig merged commit d67c9ef into stable Oct 8, 2024
221 checks passed
@simonihmig simonihmig deleted the side-watch-packages branch October 8, 2024 17:03
@github-actions github-actions bot mentioned this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants