-
Notifications
You must be signed in to change notification settings - Fork 4
fix: match variable aliases using transformed constant names #539
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: match variable aliases using transformed constant names #539
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.
Pull Request Overview
This PR extracts common variable key-to-constant name transformation logic into a reusable utility function. The refactoring consolidates duplicate logic that converts variable keys (like 'my-variable') into constant names (like 'MY_VARIABLE') and enables better handling of variable aliases in the diff command.
- Extracted
keyToConstantutility function to centralize the transformation logic - Updated
generate/types.tsto use the new utility instead of inline logic - Enhanced
diff/index.tsto handle aliased variable matching using both direct name and constant-transformed name
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/utils/keyToConstant.ts | New utility function for transforming variable keys to constant format |
| src/commands/generate/types.ts | Refactored to use the new keyToConstant utility |
| src/commands/diff/index.ts | Enhanced alias matching to support constant-transformed variable names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f7549da to
34075ee
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- /dev/null | ||
| +++ b/test-utils/fixtures/diff/sampleDiff.ts | ||
| @@ -1,1 +1,21 @@ | ||
| +const config = { |
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.
might want to call the useVariableValue here for a more accurate test instead of a string match?
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.
If we use useVariableValue then the bug doesn't get caught 🙃
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.
the bug is related to the use of the variableKey and the VariableAlias, should get caught either way
Fixes #538
The diff command couldn't match variable keys to auto-generated constant aliases.
Problem: Alias map has
MY_VARIABLE: 'my-variable', but lookup triedaliasMap['my-variable']→ undefinedSolution: Try both direct and transformed lookup:
aliasMap[name] || aliasMap[keyToConstant(name)]