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

Update logic for getting package infos and options #1022

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Nov 26, 2024

This PR fixes some inefficiencies and confused leftover logic around getPackageInfos and getting combined package options.

It appears that the original intent of beachball was to support per-package JS config files, but that actually hasn't worked since #405. But the docs were outdated on this point, and there was still some leftover code (like unnecessarily using cosmiconfig to read package-level beachball options from package.json). I don't see any immediate need for adding this back (see #1021), so this PR removes the leftovers and updates the docs.

I also noticed that getPackageInfos was re-processing the repo and CLI options (or at least calling those methods, which might be cached) once for every package. Refactoring it to only call those once might improve perf slightly.

Also due to lack of package-level JS config support, the PackageOptions level changeFilePrompt property doesn't make sense (it must be a function). So remove it and read those options from the top level instead.

@ecraig12345 ecraig12345 changed the title getPackageInfos should only get repo and CLI options once Update logic for getting package infos and options Nov 26, 2024
@ecraig12345 ecraig12345 merged commit 13d1843 into master Nov 26, 2024
4 checks passed
@ecraig12345 ecraig12345 deleted the ecraig/package-infos-perf branch November 26, 2024 02:19
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.

1 participant