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

Don't add to graph scripts missing from package.json #593

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Philip-Scott
Copy link
Member

Avoid adding packages to build graph when not specified. This will be highly impactful for our build times of our PR Deployment environment as we'd be able to just bundle packages, we deploy to this environment instead of the full repo. In our build pipelines where we need to build everything, we can add build to the Lage tasks.

Manual Validation

Within our team's repo, I was able to confirm that running lage test ran all test tasks our team has, and avoided building packages without tests and that were not in any dependency path. We went from having to build 117 packages to 108.

I can share the results with you privately to avoid sharing any private package names here :)

Fixes #592

@netlify
Copy link

netlify bot commented Mar 1, 2023

Deploy Preview for peppy-praline-1c3272 canceled.

Name Link
🔨 Latest commit e476a8d
🔍 Latest deploy log https://app.netlify.com/sites/peppy-praline-1c3272/deploys/63ffaae486bc870007266f2b

@kenotron
Copy link
Member

kenotron commented Mar 7, 2023

So, we really can't assume knowledge of whether to add something to the graph or not here in this package. We may need to factor the workspace target graph builder OUT of this package so it can know about "runners". The runners themselves tell you whether the target "shouldRun()" something. Based on that flag, then we can determine if the graph should be trimmed or not.

Copy link
Member

@kenotron kenotron left a comment

Choose a reason for hiding this comment

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

Please check my message. If you're up for it, we can chat more offline about the approach.

@kenotron
Copy link
Member

kenotron commented Mar 10, 2023

Here's what to do next:

  1. move WorkspaceTargetGraphBuilder to cli package
  2. pull in TargetRunnerPicker as a constructor dependency to the WorkspaceTargetGraphBuilder
  3. replace the conditional to use the shouldRun() function of the picked runner to skip "entries" target in WTGBuilder
  4. please add WTGBuilder test to cover the cases as we drew here:

image

@Philip-Scott
Copy link
Member Author

pipeline: {
  transpile: [],
  types: ['^types']
  copy: ['transpile']
  bundle: ['copy', '^transpile'],
  
}

root package.json {
  build: lage transpile types
}

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.

Lage scheduler adding "phantom" tasks to the graph.
2 participants