-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
fix/more validations for selected components #382
Conversation
@@ -95,7 +96,12 @@ export async function printOutput( | |||
const nativeWindUIComponents = | |||
cliResults.packages.find((p) => p.name === 'nativewindui').options.selectedComponents ?? []; | |||
|
|||
const finalComponents = Array.from(new Set([...nativeWindUIComponents, 'text'])); | |||
// we do this to account for older stored config e.g that has selectable text in it | |||
const onlySupportedComponents = nativeWindUIComponents.filter((component) => |
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.
@dannyhw i'm not sure i follow what's happening here
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.
Just filtering to real component options. For example i removed the selectable text option so if someone passes it we need to remove it
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.
Like imagine someone manually writes out the selected components they want or they saved a config before the update and it includes selectable text or a typo for example
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.
Don't we already ignore whatever is passed in that isn't a validated option?
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.
well yes but theres no validation on stored config since it just directly outputs the cliResults, so we either add a check there or here
(the local storage for configs that is in ~/.create-expo-stack/configurations.json)
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.
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.
so what if you pass in a flag like --some-bullshit-component
, will that break it too?
Description
Extra validation for when config is loaded with unsupported nwui components
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):