-
Notifications
You must be signed in to change notification settings - Fork 167
Break up gluestack-ui/src/util/index.ts grab-bag so that reuse is easier #3132
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
base: feat/v3
Are you sure you want to change the base?
Break up gluestack-ui/src/util/index.ts grab-bag so that reuse is easier #3132
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments about what this is setting up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - this was done by prettier on my system automatically.
getPackageMangerFlag(options); | ||
|
||
// If a package manager was set in options, save it to the config. | ||
savePackageManagerFromOptions(options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and in the init
command is a prime example of why this is an improvement. We are able to consolidate and reuse the code that detects and exits the process if the user has selected multiple package managers.
expoProject: 'expo', | ||
nextJsProject: 'nextjs', | ||
reactNativeCLIProject: 'react-native-cli', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These values appear to be stored here not to be configurable but just as universal constants. I've refactored them into an enum down below to represent the fact that there are only a finite number of project types and they have constant, not configurable, argument values.
} | ||
|
||
await installDependencies(updatedComponents, versionManager); | ||
await installDependencies(updatedComponents); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another example of the improvements from code encapsulation. We are now able to have installDependencies
use the same version manager detection code no matter where it's been called, improving consistency of the CLI experience across commands.
As a prerequisite for my solution to #3131, I needed to break up the grab-bag that the
src/util/index.ts
file currently is. This makes it easier to see what files are grouped together by domain, what functions are public/exported for each domain, and makes it easier to find and import just the function you're looking to reuse.