feat: apply PRs #7501, #7495, #7473#2
Open
deepshekhardas wants to merge 1 commit into
Open
Conversation
- chore: use official @module-federation/vite (refinedev#7501) - feat(core): add namespace option to useTranslation/useTranslate (refinedev#7495) - feat(antd): add onParse support to useTable and useSimpleList (refinedev#7473)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/antd/src/hooks/table/useTable/useTable.ts">
<violation number="1" location="packages/antd/src/hooks/table/useTable/useTable.ts:132">
P2: The parsed search form becomes stale after filters change while `syncWithLocation` is enabled because this effect captures the initial `onParse` and `filters` but only depends on `shouldSyncWithLocation`. Including the values used by the new branch (at least `filters`, `onParse`, and the form instance) keeps the form synchronized with the current URL filters.</violation>
</file>
<file name="examples/monorepo-module-federation/apps/blog-posts/package.json">
<violation number="1" location="examples/monorepo-module-federation/apps/blog-posts/package.json:40">
P2: The example's supported Node range is now narrower than its `engines` declaration: `@module-federation/vite@1.16.8` requires `^20.19.0 || >=22.12.0`, so installs under Node 20.0–20.18 can produce an engine failure (or fail with strict engine checks). Align the app's Node engine/documentation with the dependency or select a plugin version that supports the declared range.</violation>
</file>
<file name="packages/core/src/hooks/i18n/useTranslate.ts">
<violation number="1" location="packages/core/src/hooks/i18n/useTranslate.ts:31">
P2: Calls such as `translate("key", "Fallback")` can stop returning the fallback with existing providers because this normalization moves the string from the provider's second argument to its third. Preserving the original two-argument call when `ns` is absent avoids this compatibility regression.</violation>
<violation number="2" location="packages/core/src/hooks/i18n/useTranslate.ts:41">
P2: The hook's namespace is omitted for the valid call `translate(key, undefined, defaultMessage)` because this guard requires a truthy `options` value. Merging whenever `options` is not a string keeps the configured namespace active when no interpolation options are supplied.</violation>
</file>
<file name="examples/monorepo-module-federation/apps/host/package.json">
<violation number="1" location="examples/monorepo-module-federation/apps/host/package.json:38">
P2: This dependency narrows the supported Node 20 range to `>=20.19.0`, but the host app still advertises `>=20`; align the app/example engine declarations with the plugin requirement or select a plugin version that supports the advertised Node range.</violation>
</file>
<file name="packages/antd/src/hooks/useSimpleList/useSimpleList.ts">
<violation number="1" location="packages/antd/src/hooks/useSimpleList/useSimpleList.ts:115">
P2: The new `useEffect` for syncing URL filters to the search form references `filters`, `onParse`, and `form` inside its body but only lists `shouldSyncWithLocation` in the dependency array. If `filters` change while `shouldSyncWithLocation` stays true (e.g., after a browser back/forward navigation that changes URL params but keeps syncWithLocation enabled), the effect won't re-run and the form won't reflect the updated filter values. The `onParse` function reference could also become stale if it's recreated on re-renders. Add the referenced values to the dependency array to keep the form in sync with filter changes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // set values to form | ||
| formSF.form.setFieldsValue(filterFilterMap as any); | ||
| if (onParse) { | ||
| const parsedValues = onParse(filters); |
There was a problem hiding this comment.
P2: The parsed search form becomes stale after filters change while syncWithLocation is enabled because this effect captures the initial onParse and filters but only depends on shouldSyncWithLocation. Including the values used by the new branch (at least filters, onParse, and the form instance) keeps the form synchronized with the current URL filters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/antd/src/hooks/table/useTable/useTable.ts, line 132:
<comment>The parsed search form becomes stale after filters change while `syncWithLocation` is enabled because this effect captures the initial `onParse` and `filters` but only depends on `shouldSyncWithLocation`. Including the values used by the new branch (at least `filters`, `onParse`, and the form instance) keeps the form synchronized with the current URL filters.</comment>
<file context>
@@ -126,28 +128,33 @@ export const useTable = <
- // set values to form
- formSF.form.setFieldsValue(filterFilterMap as any);
+ if (onParse) {
+ const parsedValues = onParse(filters);
+ formSF.form.setFieldsValue(parsedValues as any);
+ } else {
</file context>
| }, | ||
| "devDependencies": { | ||
| "@originjs/vite-plugin-federation": "^1.2.3", | ||
| "@module-federation/vite": "^1.16.8", |
There was a problem hiding this comment.
P2: The example's supported Node range is now narrower than its engines declaration: @module-federation/vite@1.16.8 requires ^20.19.0 || >=22.12.0, so installs under Node 20.0–20.18 can produce an engine failure (or fail with strict engine checks). Align the app's Node engine/documentation with the dependency or select a plugin version that supports the declared range.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/monorepo-module-federation/apps/blog-posts/package.json, line 40:
<comment>The example's supported Node range is now narrower than its `engines` declaration: `@module-federation/vite@1.16.8` requires `^20.19.0 || >=22.12.0`, so installs under Node 20.0–20.18 can produce an engine failure (or fail with strict engine checks). Align the app's Node engine/documentation with the dependency or select a plugin version that supports the declared range.</comment>
<file context>
@@ -37,7 +37,7 @@
},
"devDependencies": {
- "@originjs/vite-plugin-federation": "^1.2.3",
+ "@module-federation/vite": "^1.16.8",
"@types/node": "^20",
"@types/react": "^19.1.0",
</file context>
| } | ||
| } | ||
|
|
||
| if (ns && options && typeof options !== "string") { |
There was a problem hiding this comment.
P2: The hook's namespace is omitted for the valid call translate(key, undefined, defaultMessage) because this guard requires a truthy options value. Merging whenever options is not a string keeps the configured namespace active when no interpolation options are supplied.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/hooks/i18n/useTranslate.ts, line 41:
<comment>The hook's namespace is omitted for the valid call `translate(key, undefined, defaultMessage)` because this guard requires a truthy `options` value. Merging whenever `options` is not a string keeps the configured namespace active when no interpolation options are supplied.</comment>
<file context>
@@ -21,20 +22,37 @@ export const useTranslate = () => {
+ }
+ }
+
+ if (ns && options && typeof options !== "string") {
+ options = { ns, ...options };
+ }
</file context>
Suggested change
| if (ns && options && typeof options !== "string") { | |
| if (ns && typeof options !== "string") { |
Comment on lines
+31
to
+39
| if (typeof options === "string" && defaultMessage === undefined) { | ||
| defaultMessage = options; | ||
|
|
||
| if (ns) { | ||
| options = { ns }; | ||
| } else { | ||
| options = undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
P2: Calls such as translate("key", "Fallback") can stop returning the fallback with existing providers because this normalization moves the string from the provider's second argument to its third. Preserving the original two-argument call when ns is absent avoids this compatibility regression.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/hooks/i18n/useTranslate.ts, line 31:
<comment>Calls such as `translate("key", "Fallback")` can stop returning the fallback with existing providers because this normalization moves the string from the provider's second argument to its third. Preserving the original two-argument call when `ns` is absent avoids this compatibility regression.</comment>
<file context>
@@ -21,20 +22,37 @@ export const useTranslate = () => {
+ let options = _options;
+ let defaultMessage = _defaultMessage;
+
+ if (typeof options === "string" && defaultMessage === undefined) {
+ defaultMessage = options;
+
</file context>
Suggested change
| if (typeof options === "string" && defaultMessage === undefined) { | |
| defaultMessage = options; | |
| if (ns) { | |
| options = { ns }; | |
| } else { | |
| options = undefined; | |
| } | |
| } | |
| if (typeof options === "string" && defaultMessage === undefined && ns) { | |
| defaultMessage = options; | |
| options = { ns }; | |
| } |
| }, | ||
| "devDependencies": { | ||
| "@originjs/vite-plugin-federation": "^1.2.3", | ||
| "@module-federation/vite": "^1.16.8", |
There was a problem hiding this comment.
P2: This dependency narrows the supported Node 20 range to >=20.19.0, but the host app still advertises >=20; align the app/example engine declarations with the plugin requirement or select a plugin version that supports the advertised Node range.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/monorepo-module-federation/apps/host/package.json, line 38:
<comment>This dependency narrows the supported Node 20 range to `>=20.19.0`, but the host app still advertises `>=20`; align the app/example engine declarations with the plugin requirement or select a plugin version that supports the advertised Node range.</comment>
<file context>
@@ -35,7 +35,7 @@
},
"devDependencies": {
- "@originjs/vite-plugin-federation": "^1.2.3",
+ "@module-federation/vite": "^1.16.8",
"@types/node": "^20",
"@types/react": "^19.1.0",
</file context>
| const { syncWithLocation: defaultSyncWithLocation } = useSyncWithLocation(); | ||
| const shouldSyncWithLocation = syncWithLocation ?? defaultSyncWithLocation; | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
P2: The new useEffect for syncing URL filters to the search form references filters, onParse, and form inside its body but only lists shouldSyncWithLocation in the dependency array. If filters change while shouldSyncWithLocation stays true (e.g., after a browser back/forward navigation that changes URL params but keeps syncWithLocation enabled), the effect won't re-run and the form won't reflect the updated filter values. The onParse function reference could also become stale if it's recreated on re-renders. Add the referenced values to the dependency array to keep the form in sync with filter changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/antd/src/hooks/useSimpleList/useSimpleList.ts, line 115:
<comment>The new `useEffect` for syncing URL filters to the search form references `filters`, `onParse`, and `form` inside its body but only lists `shouldSyncWithLocation` in the dependency array. If `filters` change while `shouldSyncWithLocation` stays true (e.g., after a browser back/forward navigation that changes URL params but keeps syncWithLocation enabled), the effect won't re-run and the form won't reflect the updated filter values. The `onParse` function reference could also become stale if it's recreated on re-renders. Add the referenced values to the dependency array to keep the form in sync with filter changes.</comment>
<file context>
@@ -106,6 +109,38 @@ export const useSimpleList = <
+ const { syncWithLocation: defaultSyncWithLocation } = useSyncWithLocation();
+ const shouldSyncWithLocation = syncWithLocation ?? defaultSyncWithLocation;
+
+ useEffect(() => {
+ if (shouldSyncWithLocation) {
+ if (onParse) {
</file context>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Three more from refinedev/refine:
Summary by cubic
Switches examples to the official
@module-federation/viteplugin and adds i18n namespace support plus better URL→form syncing in@refinedev/antdhooks. Also enhances document title i18n handling.New Features
@refinedev/core:useTranslationanduseTranslateaccept{ ns }to scope translations.@refinedev/antd:useTableanduseSimpleListsupportonParse(filters)to map URL-synced filters into search form values; tests added.@refinedev/react-router:useDocumentTitle(title, { ns, defaultTitle })for namespaced titles and a fallback.Dependencies
@originjs/vite-plugin-federationwith@module-federation/vite; use{ federation }, setfilename: "remoteEntry.js",dts: false.VITE_CATEGORIES_URLvialoadEnv; remove window mutation andtypes.ts; update.env.exampletoremoteEntry.js.react-router(fromreact-router-dom) in module federation configs.Written for commit 92507f0. Summary will update on new commits.