-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
refactor: replace const enum
with enum
#9263
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
Conversation
Size ReportBundles
Usages
|
How can I rerun e2e-test? It passed on my machine 😬. |
The js generated by your PR will look like this var ElementTypes = /* @__PURE__ */ ((ElementTypes2) => {
ElementTypes2[ElementTypes2["ELEMENT"] = 0] = "ELEMENT";
ElementTypes2[ElementTypes2["COMPONENT"] = 1] = "COMPONENT";
ElementTypes2[ElementTypes2["SLOT"] = 2] = "SLOT";
ElementTypes2[ElementTypes2["TEMPLATE"] = 3] = "TEMPLATE";
return ElementTypes2;
})(ElementTypes || {});
If we change it to const ElementTypes = {
ELEMENT: 0,
COMPONENT: 1,
SLOT: 2,
TEMPLATE: 3,
0: 'ELEMENT',
1: 'COMPONENT',
2: 'SLOT',
3: 'TEMPLATE',
}; If will help library users to use it like const enum with tree shaking help. Especially for some larger enums, such as ErrorCodes, which have more than 50 enum members, the size problem will be very serious. Can you modify your code to generate constObject style enum? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add eslint rules as well?
See https://github.com/vuejs/core/pull/9261/files#diff-e1ce717e1179a0db14e90ec5374768a206651ca807db58352991eb7895ed7c9c
6bc0415
to
3fe838c
Compare
@xiaoxiangmoe Hi, are you interested in collaborating? Let's merge our PRs |
It seems like current there is no easy way to transform Let's see if esbuild can help. evanw/esbuild#3402 |
esbuild refused, which was not surprising. If Vue want transform But since there is no simple way to do this, is it worth it? |
@yangmingshan No, there will have no type and runtime mismatch. See my latest update ![]() |
@xiaoxiangmoe The benefits of your approach are:
Right? |
Yes. |
Better is better, complexity is no excuse for libraries. I prefer @xiaoxiangmoe's PR, I'm closing this one. |
Fixes #1228
As we discussed in #9243
Turns out we don't need a rollup plugin, global replacements is enough.