-
Notifications
You must be signed in to change notification settings - Fork 4
Revert "fix: match variable aliases using transformed constant names" #542
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
Revert "fix: match variable aliases using transformed constant names" #542
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 removes the auto-alias transformation feature that attempted to resolve variable aliases by transforming constant names (e.g., MY_VARIABLE -> my-variable). The change simplifies alias resolution to rely solely on direct matches from the generated types file.
- Inlined the
keyToConstantutility function directly into the types generator - Removed the fallback transformation logic from the diff command's alias resolution
- Cleaned up test fixtures and test cases related to the auto-alias transformation feature
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test-utils/fixtures/diff/auto-alias-transform | Removed test fixture file for auto-alias transformation feature |
| test-utils/fixtures/configs/mockTypes.ts | Removed mock types file used for testing auto-alias transformation |
| test-utils/fixtures/configs/autoAliasConfig.yml | Removed configuration file for auto-alias transformation tests |
| src/utils/keyToConstant.ts | Removed utility function as it's now inlined in the types generator |
| src/commands/generate/types.ts | Inlined the transformation logic and added lodash import |
| src/commands/diff/index.ts | Simplified alias resolution by removing transformation fallback |
| src/commands/diff/diff.test.ts | Removed test case for auto-alias transformation feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| Object.values(matchesBySdk).forEach((matches) => { | ||
| matches.forEach((m) => { | ||
| const match = { ...m } |
Copilot
AI
Nov 6, 2025
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 removal of the keyToConstant transformation fallback means that aliases must now match exactly. Consider documenting this behavior change in the codebase or user-facing documentation to help users understand that the generated constant names from the types file must be used directly, without any automatic kebab-case transformation.
| const match = { ...m } | |
| const match = { ...m } | |
| // NOTE: The removal of the keyToConstant transformation fallback means that aliases must now match exactly. | |
| // The generated constant names from the types file must be used directly, without any automatic kebab-case transformation. |
|
|
||
| getVariableGeneratedName(variable: Variable) { | ||
| let constantName = keyToConstant(variable.key) | ||
| let constantName = upperCase(variable.key).replace(/\s/g, '_') |
Copilot
AI
Nov 6, 2025
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.
[nitpick] This logic is now duplicated from the removed keyToConstant utility. While the utility was only used in one other place, consider extracting this transformation to a helper function within this file (e.g., private keyToConstant(key: string)) to make the transformation logic more explicit and maintainable, especially since it's a critical part of the variable name generation.
Reverts #539