-
Notifications
You must be signed in to change notification settings - Fork 418
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
feat(cli): use auto-updates flag in init #7401
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
Component Testing Report Updated Sep 11, 2024 11:05 PM (UTC)
|
5538e15
to
9d1dc0a
Compare
9d1dc0a
to
69395f5
Compare
69395f5
to
2acba13
Compare
2acba13
to
9c5d788
Compare
if (typeof value !== 'boolean') { | ||
throw new Error(`Expected boolean value for '${variableName}'`) | ||
} | ||
node.name = value.toString() |
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.
I don't like this approach - we're relying on the fact that babel won't validate that the name is valid. This identifier node should be replaced with a Literal node. I'll do a fix.
6deceda
to
2e92243
Compare
2e92243
to
bd5c9ad
Compare
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.
looks good! i believe this should result in right CLI config
not sure if this was suggested yet, but it seems like we're reimplementing a lot of what babel template could solve but it may be more complicated with dependencies, typescript support, etc so feel free to ignore
Description
As we move to make auto-updates the default way of updating studios, we want to ensure that we give developers the option to maintain legacy behavior and keep to specific versions if they prefer.
This PR updates the
sanity init
command to allow a--no-auto-updates
flag that will then be injected into the CLI configuration asautoUpdates: true
orautoUpdates: false
This required a bit of refactoring of our template logic, since we previously only dealt with string variables. I've added another way of identifying the variables we're injecting, but happy to refactor if this is not ideal.
What to review
Are there any ways in which auto-updates might be undefined? Any gotchas in the AST parser?
Testing
I've added a test for this against Typescript -- if needed, I can repeat those tests for the other cases (Javascript, templates) but assume all will be consistent.
(No release notes because this feature is still in beta)