-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Additional top-level minification #3570
Comments
Ah, although I just realized it does work within a function: https://esbuild.github.io/try/#dAAwLjE5LjExAC0tbWluaWZ5AChmdW5jdGlvbigpIHsKY29uc3QgbyA9ICdvJzsKY29uc3QgZCA9ICdkJzsKY29uc3QgYm9vbGVhbiA9IGZhbHNlOwp2YXIgZnJhZyA9IGA8cCBhdXRvY2FwaXRhbGl6ZT0iJHtgdyR7b31yJHtkfXNgfSIgY29udGVudGVkaXRhYmxlPSIke2Jvb2xlYW59Ij4gPC9wPmA7CmNvbnNvbGUubG9nKGZyYWcpOwp9KSgp So it seems like what I really want is something like terser's I was a bit confused by the reference to Svelte in #3568 (comment) as a reason for not doing it by default. I'm a Svelte maintainer, and as far as I'm aware we don't do this and the other Svelte maintainers I've asked also aren't quite sure what it might be referring to. So if you've been avoiding it on our behalf, I'm not sure you need to. In fact I'm requesting this to better minify the output of the Svelte compiler where top-level nested template strings being spit out like like this are not uncommon |
--toplevel
option similar to terser for additional minification
My Svelte comment in #3568 (comment) was talking about this behavior: #604. The code esbuild sees is not the entire module. In particular, the surrounding HTML expects to be able to reference otherwise unused variables in the script tag. From what I understand, this is because Svelte uses esbuild to process the code inside the script tag, which is a module fragment, and then concatenates it with additional generated code to create the full module. Please correct me if I’m misunderstanding something. |
Ah, thank you for the pointer! It appears that issue isn't related to Svelte itself, but a particular plugin svelte-preprocess-esbuild. The Svelte compiler itself only knows how to compile JS/CSS code and so we have "preprocessors", which convert things like PostCSS and TypeScript to JS and CSS. The most used officially supported solution (known as So tl;dr, please feel free to be more aggressive if |
--toplevel
option similar to terser for additional minification
I think For Svetle, maybe a different format can be created (e.g. |
As a Svelte maintainer, I would say there is zero need to do anything special for Svelte here |
I was going to send a PR to revert the special behavior for Svelte, but I don't think it would actually help in this case. The special Svelte behavior is supposed to only take case when esbuild/internal/js_parser/js_parser.go Line 16552 in 3327274
But if I pass that flag, things still aren't being minimized when at the top level: |
@benmccann The code you referenced is used to "scan imports and exports", which doesn't do anything about drop plain variables. There're 2 different variables need to be preserved inside svelte code fragments (import names and unused variables) and in esbuild it is controlled by different aspects (both in transform mode):
So back to your initial question. If you are processing the entire code (not a piece of code fragment like in svelte), and want to minify and drop some unused variables' names in transform mode, just enable "--format={your code format} --minify --tree-shaking". However, string literals aren't inlined in string templates for now (as #3568 doesn't do this). |
The really confusing thing is that sometimes they are! If you look at this example, it will inline the variable if the template is in a function, but not if the template is at the top-level. I don't really have an explanation for this. It seemed to be suggested that opting out was for the benefit of Svelte, but - as you've pointed out - the Svelte-specific code is totally unrelated to such transformations. So I'm not quite sure why this behavior exists, but would love if it's possible to align the top-level behavior with the behavior inside a function |
@benmccann Yes you're right. I guess I know what happened here: As pointed above, one of the constant inlining strategy is to inline top const numbers (null, boolean, etc. but no string), example. So why do you see const strings inlined inside functions? This is because it invokes another process: Inline single-use variable declarations, example. It doesn't apply to top-level constants because the parser at this time don't know if there be further |
Ah, I see! How feasible does this seem to implement? Would a second pass need to be made and is that something that would be considered? |
Although the comments say
I guess this still can be checked since you have all statements in some scope. So for every |
Here
boolean
is inlined, buto
andd
are not. I believe all three variables should be able to be inlinedhttps://esbuild.github.io/try/#dAAwLjE5LjExAC0tbWluaWZ5AGNvbnN0IG8gPSAnbyc7CmNvbnN0IGQgPSAnZCc7CmNvbnN0IGJvb2xlYW4gPSBmYWxzZTsKdmFyIGZyYWcgPSBgPHAgYXV0b2NhcGl0YWxpemU9IiR7YHcke299ciR7ZH1zYH0iIGNvbnRlbnRlZGl0YWJsZT0iJHtib29sZWFufSI+IDwvcD5gOw
The text was updated successfully, but these errors were encountered: