Skip to content
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

fix(compiler-core): simple identifier should include Chinese #10575

Closed
wants to merge 2 commits into from

Conversation

wangyewei
Copy link
Contributor

close #10542

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 90.6 kB 34.4 kB 31.1 kB
vue.global.prod.js 148 kB (+13 B) 53.7 kB (+14 B) 47.9 kB (-34 B)

Usages

Name Size Gzip Brotli
createApp 50.8 kB 19.8 kB 18.1 kB
createSSRApp 54.1 kB 21.2 kB 19.3 kB
defineCustomElement 53.1 kB 20.6 kB 18.8 kB
overall 64.4 kB 24.9 kB 22.6 kB

@@ -62,7 +62,7 @@ export function isCoreComponent(tag: string): symbol | void {
}
}

const nonIdentifierRE = /^\d|[^\$\w]/
const nonIdentifierRE = /^\d|[^\$\w\u4e00-\u9fa5]/
Copy link
Member

Choose a reason for hiding this comment

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

please add unit test 😁

@baiwusanyu-c baiwusanyu-c added the need test The PR has missing test cases. label Mar 23, 2024
@edison1105
Copy link
Member

duplicate of #6765

Comment on lines +65 to 67
const nonIdentifierRE = /^\d|[^\$\w\u4e00-\u9fa5]/
export const isSimpleIdentifier = (name: string): boolean =>
!nonIdentifierRE.test(name)
Copy link
Member

Choose a reason for hiding this comment

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

I think we can refactor this part by using propNameEscapeSymbolsRE (move it to shared pkg)

export const propNameEscapeSymbolsRE = /[ !"#$%&'()*+,./:;<=>?@[\\\]^`{|}~\-]/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems that directly use this regx to instead will break other test units

@baiwusanyu-c baiwusanyu-c removed the need test The PR has missing test cases. label Mar 26, 2024
@skirtles-code
Copy link
Contributor

#6765 has now been merged. The fix and test in that PR are slightly different from those found here. I'm unclear whether this PR is still worth pursuing, but it will need the conflicts resolving if it is.

@edison1105
Copy link
Member

Close as duplicate

@edison1105 edison1105 closed this Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants