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.

Sign up for GitHub

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

Jump to bottom

feat(compiler-core): support Symbol in template #9069

Merged
merged 10 commits into from
May 27, 2024
Merged

feat(compiler-core): support Symbol in template #9069

merged 10 commits into from
May 27, 2024

Conversation

4xii
Copy link
Contributor

@4xii 4xii commented Aug 29, 2023 •
edited by sxzz
Loading

relate #9027

@github-actions
Copy link

github-actions bot commented Aug 29, 2023 •
edited
Loading

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 94.1 kB (+3.29 kB) 35.5 kB (+1.02 kB) 32 kB (+913 B)
vue.global.prod.js 151 kB (+3.29 kB) 54.8 kB (+1.06 kB) 48.9 kB (+912 B)

Usages

Name Size Gzip Brotli
createApp 54 kB (+3.24 kB) 20.9 kB (+1.03 kB) 19 kB (+824 B)
createSSRApp 57.4 kB (+3.25 kB) 22.2 kB (+1 kB) 20.2 kB (+859 B)
defineCustomElement 56.3 kB (+3.25 kB) 21.7 kB (+1.05 kB) 19.6 kB (+811 B)
overall 67.8 kB (+3.23 kB) 25.9 kB (+983 B) 23.4 kB (+809 B)

@Justineo
Copy link
Member

I think our goal is not to make it as comprehensive as possible, but sufficient to meet actual needs. I don't think there are actual use cases for most of the added globals here.

sxzz
sxzz previously approved these changes Aug 31, 2023
View reviewed changes
Copy link
Member

@sxzz sxzz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM in code perspective, waiting for more discussion.

  • necessary?
  • minor?

@sxzz sxzz changed the title feat: refactor globalsAllowList.ts to use a more comprehensive list of globally allowed values feat(shared): more comprehensive list for globally allowed Aug 31, 2023
@sxzz sxzz dismissed their stale review September 5, 2023 08:57

need changes

shixinzhu added 2 commits September 6, 2023 13:15
feat: update globalsAllowList.ts with simplified global variablesThis…
5112137 
… commit removes obsolete global variables from the globalsAllowList.ts file to simplify and optimize the code
feat: refactor globalsAllowList.ts for improved readability and maint…
4471c54 
…ainability
@sxzz sxzz added ready to merge The PR is ready to be merged. and removed wait changes labels Sep 6, 2023
@sxzz sxzz mentioned this pull request Oct 20, 2023
Merged
@sxzz sxzz changed the title feat(shared): more comprehensive list for globally allowed feat(compiler): add support for Symbol global Oct 20, 2023
@sxzz
Copy link
Member

sxzz commented Oct 20, 2023 •
edited
Loading

merge PR #7018 first.

@sxzz sxzz changed the title feat(compiler): add support for Symbol global feat(compiler-core): support Symbol in template Oct 20, 2023
sxzz
sxzz requested changes Oct 20, 2023
View reviewed changes
Copy link
Member

@sxzz sxzz left a comment

Choose a reason for hiding this comment

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

A unit test is needed in packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts

@sxzz sxzz added the need test The PR has missing test cases. label Oct 20, 2023
@sxzz sxzz removed the ready to merge The PR is ready to be merged. label Oct 20, 2023
@yyx990803 yyx990803 changed the base branch from main to minor May 27, 2024 09:13
@yyx990803 yyx990803 merged commit a501a85 into vuejs:minor May 27, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sxzz sxzz sxzz requested changes

Assignees
No one assigned
Labels
need test The PR has missing test cases. scope: compiler
Projects
Issue / PR Management
Status: Merged
Triage Session
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@4xii @Justineo @sxzz @yyx990803