fix: address codex-bot follow-ups for 2026-03-21 merged PRs#600
Conversation
|
Code ReviewOverall this is a solid follow-up PR with good test coverage. A few items worth flagging: Potential Issues1.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d7d66c94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let is_match = if signature.from.is_none() { | ||
| default_spec.local.sym == signature.name | ||
| } else { | ||
| signature.is_default_specifier() | ||
| || default_spec.local.sym == signature.name |
There was a problem hiding this comment.
name: "default"
When a signature has a concrete from and a non-default name, this branch still treats any default import whose local binding happens to equal signature.name as a match. With a config like { from: "my-loadable", name: "lazy" }, import lazy from "my-loadable" will now be rewritten even though only import { lazy } ... should match; the README and Signature::is_default_specifier() still reserve name: "default" for default imports. That changes runtime behavior for wrappers that expose both a default export and named helpers.
Useful? React with 👍 / 👎.
| JSXElementName::JSXMemberExpr(member_expr) => { | ||
| if !self.component_names.contains(&*member_expr.prop.sym) { | ||
| return; |
There was a problem hiding this comment.
This now extracts any JSX member expression whose final property name matches a tracked component, without checking what object it comes from. In projects that render unrelated components such as <UI.FormattedMessage> or <Messages.FormattedMessage>, the visitor will start deleting description and injecting an id even though those components are not from react-intl. Before this change, those member-expression components were left alone, so this is a new false-positive path.
Useful? React with 👍 / 👎.
Code ReviewThanks for this follow-up PR addressing multiple bot review items. The changes are well-structured with good test coverage. Here is my analysis by package. graphql-codegen-client-preset** (new helper) — looks good**
Behavior change: unknown conventions now preserve original name // Before
_ => to_pascal_case(s),
// After
_ => s.to_string(),This is more correct, but it is a silent breaking change for users who were relying on the old fall-through PascalCase behavior with a custom convention string. Consider documenting this in the PR description or a CHANGELOG entry. Minor style nit: The packages/emotion
Span change for PURE comment placement Namespace import support for packages/formatjsJSXMemberExpr matching
packages/loadable-components
The fixture at Aliased import fix let is_match = if signature.from.is_none() {
default_spec.local.sym == signature.name // must match configured local name
} else {
signature.is_default_specifier() || default_spec.local.sym == signature.name
};This prevents over-matching unrelated default imports for source-less signatures. The fixture **Signature name change ( README updatesThe jest README change is accurate — the plugin is specifically a Jest mock-hoisting transform, not a general Summary
Overall this is a high-quality follow-up. No blocking issues found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a210f2df39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| PackageMeta::Namespace(module) => module | ||
| .exported_names | ||
| .iter() | ||
| .find(|item| matches!(item.kind, ExprKind::Css)) | ||
| .map(|item| CssPropCallee::Namespace { |
There was a problem hiding this comment.
css export
In the new namespace-import css-prop path, this picks the first export whose kind is ExprKind::Css, but that enum is also used for keyframes aliases (packages/emotion/transform/src/import_map.rs:43-51). If a project uses an import-map package with import * as emo from "pkg" and the only mapped emotion-style export is keyframes, <div css={{...}} /> will now be rewritten to emo.keyframes(...), which returns an animation name instead of serialized styles. The namespace match here needs to require an actual css export, not any ExprKind::Css entry.
Useful? React with 👍 / 👎.
1 participant
Summary
chatgpt-codex-connector[bot]review follow-ups across merged PRs: fix(loadable-components): Support aliased loadable imports #585, fix(formatjs): scope static evaluation to formatjs descriptor keys only #591, fix(loadable-components): skip transformation when ssr: false is specified #592, fix(graphql-codegen-client-preset): respect namingConvention config #593, test(formatjs): Add regression test for issue #532 string concatenation in defaultMessage #594, fix(graphql-codegen-client-preset): handle Windows-style paths in WASM runtime #595, fix(emotion): fix autoLabel not adding label to css tagged template class names #597, docs: add descriptions to plugin README.tmpl.md files #598, feat(emotion): add sourcemap and auto-label for css prop with object styles #599.What Changed
loadable-components)ssr: falsedetection to respect final object-literal override order.formatjs)ReactIntl.FormattedMessage).ast: truepath.graphql-codegen-client-preset)namingConventionparsing to accept string/object forms.keep/unknown conventions instead of forcing PascalCase.filenamepath handling in WASM runtime path resolution.emotion)emotionReact.css).packages/jest/README.tmpl.mdandpackages/swc-sdk/README.tmpl.md.Validation
cargo test -p swc_plugin_loadable_components --test fixture -- --ignoredcargo test -p swc_plugin_graphql_codegen_client_presetcargo test -p swc_emotion --test fixture -- --ignoredpnpm -C /Users/kdy1/.codex/worktrees/17e6/plugins/packages/formatjs testAll passed (formatjs has an existing non-blocking Vitest warning about an un-awaited rejects assertion).