-
Notifications
You must be signed in to change notification settings - Fork 646
Revert "chore(build): remove code property from generated components.json" #7262
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 "chore(build): remove code property from generated components.json" #7262
Conversation
|
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
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 reverts PR #7257, which had removed the code property from generated components.json. The revert restores the code property to story objects and changes the build script execution method from node to tsx to handle JSON imports without import assertions.
Key Changes
- Re-adds the
codeproperty to story objects in the generated components.json output - Removes JSON import assertions (
with {type: 'json'}) from build scripts - Updates npm scripts to use
tsxinstead ofnodefor executing TypeScript build scripts
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/react/script/hooks-json/build.ts | Removes JSON import assertions to use tsx-compatible imports |
| packages/react/script/components-json/build.ts | Removes JSON import assertions and restores code property to story objects at lines 163, 175, and 187 |
| packages/react/package.json | Changes build scripts from node to tsx for TypeScript execution |
| // Add default story to the beginning of the array | ||
| if (defaultStoryCode) { | ||
| const hasDefaultStory = docs.stories.find(story => story.id === defaultStoryId) | ||
| const hasDefaultStory = docs.stories.find(story => story.code === defaultStoryCode) |
Copilot
AI
Dec 2, 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.
This logic for checking if a default story already exists is incorrect. It's comparing story.code === defaultStoryCode (checking if the code content matches), but it should be comparing story.id === defaultStoryId (checking if the story ID matches).
This could lead to duplicate stories with the same ID if the code content doesn't match exactly, or it could incorrectly skip adding the default story if a different story happens to have identical code.
Suggested fix:
const hasDefaultStory = docs.stories.find(story => story.id === defaultStoryId)| const hasDefaultStory = docs.stories.find(story => story.code === defaultStoryCode) | |
| const hasDefaultStory = docs.stories.find(story => story.id === defaultStoryId) |
llastflowers
left a comment
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.
ty! ✨
Reverts #7257