Skip to content

fix: cannot resolve the ast messages which has json path for v11 #2159

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

Merged
merged 1 commit into from
Apr 5, 2025

Conversation

kazupon
Copy link
Member

@kazupon kazupon commented Apr 5, 2025

ref #2156

@kazupon kazupon requested a review from Copilot April 5, 2025 17:14
@kazupon kazupon added the Type: Bug Bug or Bug fixes label Apr 5, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

packages/vue-i18n-core/src/utils.ts:97

  • [nitpick] The conditional logic that checks for a message AST in the parent object and then conditionally copies or deletes properties could be clarified. Consider refactoring this branch to explicitly separate the handling of message ASTs from non-AST objects to avoid potential unintended deletions.
if (!hasStringValue) { ... }

packages/core-base/src/resolver.ts:354

  • [nitpick] Returning null immediately when encountering an AST node property key on a message AST may be counterintuitive. Consider adding detailed inline documentation or refactoring the check to make the rationale for skipping these keys clearer for future maintainers.
if (AST_NODE_PROPS_KEYS.includes(key) && isMessageAST(last)) {

Copy link

Deploying vue-i18n-next with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9faf3fa
Status: ✅  Deploy successful!
Preview URL: https://a9a20bc2.vue-i18n-next.pages.dev
Branch Preview URL: https://fix-resolve-ast-message-v11.vue-i18n-next.pages.dev

View logs

@kazupon kazupon changed the title fix: cannot resolve the ast messages which has json path fix: cannot resolve the ast messages which has json path for v11 Apr 5, 2025
Copy link

pkg-pr-new bot commented Apr 5, 2025

Open in StackBlitz

@intlify/core

npm i https://pkg.pr.new/@intlify/core@2159

@intlify/core-base

npm i https://pkg.pr.new/@intlify/core-base@2159

@intlify/devtools-types

npm i https://pkg.pr.new/@intlify/devtools-types@2159

@intlify/message-compiler

npm i https://pkg.pr.new/@intlify/message-compiler@2159

petite-vue-i18n

npm i https://pkg.pr.new/petite-vue-i18n@2159

@intlify/shared

npm i https://pkg.pr.new/@intlify/shared@2159

vue-i18n

npm i https://pkg.pr.new/vue-i18n@2159

@intlify/vue-i18n-core

npm i https://pkg.pr.new/@intlify/vue-i18n-core@2159

commit: 9faf3fa

@kazupon kazupon merged commit 2b9e5a9 into v11 Apr 5, 2025
26 checks passed
@kazupon kazupon deleted the fix/resolve-ast-message-v11 branch April 5, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug or Bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant