Fix race condition with setExperimentalFeatures #8637
+142
−46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
pierremtb
commented
Oct 22, 2025
| cmdBar, | ||
| }) => { | ||
| const initialCode = `@settings(defaultLengthUnit = in, experimentalFeatures = allow) | ||
| const initialCode = `@settings(defaultLengthUnit = in) |
There was a problem hiding this comment.
This is how I discovered this in #8565
pierremtb
commented
Oct 22, 2025
| toast.success( | ||
| `Updated file experimental features level to ${level.type}` | ||
| ) | ||
| const newAst = setExperimentalFeatures(level) |
There was a problem hiding this comment.
Changing this to act like other code mods, returning the new ast to be updated the regular way, instead of just a code execution.
pierremtb
commented
Oct 22, 2025
Comment on lines
3783
to
3791
| // Remove once this command isn't experimental anymore | ||
| let astWithNewSetting: Node<Program> | undefined | ||
| if (kclManager.fileSettings.experimentalFeatures?.type !== 'Allow') { | ||
| const result = await setExperimentalFeatures({ type: 'Allow' }) | ||
| if (err(result)) { | ||
| return Promise.reject(result) | ||
| const ast = setExperimentalFeatures({ type: 'Allow' }) | ||
| if (err(ast)) { | ||
| return Promise.reject(ast) | ||
| } | ||
|
|
||
| astWithNewSetting = ast |
There was a problem hiding this comment.
This was the problematic part in Hole. Now, since we return an ast like other codemods, we don't even have to run two executions here, but rather pass it directly to the actual codemod, see below
pierremtb
commented
Oct 22, 2025
Comment on lines
-156
to
-167
|
|
||
| export async function setExperimentalFeatures( | ||
| level: WarningLevel | ||
| ): Promise<void | Error> { | ||
| const newCode = changeExperimentalFeatures(codeManager.code, level) | ||
| if (err(newCode)) { | ||
| return new Error(`Failed to set experimental features: ${newCode.message}`) | ||
| } | ||
| codeManager.updateCodeStateEditor(newCode) | ||
| await codeManager.writeToFile() | ||
| await kclManager.executeCode() | ||
| } |
There was a problem hiding this comment.
Moved to settings.ts
pierremtb
commented
Oct 22, 2025
| * Change the meta settings for the kcl file. | ||
| * @returns the new kcl string with the updated settings. | ||
| */ | ||
| export function changeExperimentalFeatures( |
There was a problem hiding this comment.
Not changing anything here, but making it possible to run in vitest
nadr0
reviewed
Oct 22, 2025
| describe('settings.spec.ts', () => { | ||
| describe('Testing setExperimentalFeatures', () => { | ||
| it('should add the annotation and set the flag if not present', async () => { | ||
| const instance = await loadAndInitialiseWasmInstance(WASM_PATH) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #8636, needed for #8565
And make it vitest testable!