You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently when using radix-vue in a project that is built using esbuild tree shaking does not work very well due to various reasons. This can be easily seen by bundling a simple file like:
I can submit a PR with this change, but I chose to open an issue since I don't know if this is the way you would like to fix this issue. Alternatively the popper props can just be put right into the defineProps statement: const props = withDefaults(defineProps<PopperContentProps>(), PopperContentPropsDefaultValue) but that excludes the possibility of adding more props in the future to MenuContentImpl that do not also apply to PopperContent - tho maybe that's okay?
Another treeshaking issue comes from the use of createContext. Since createContext is used inside plain <script> tags in .vue files; export const [injectPopperContentContext, providePopperContentContext] = createContext<PopperContentContext>('PopperContent'). The statement will be added verbatim in the bundle (although minified), and since it is a destructure statement esbuild will not be able to treeshake it. I propose instead to export an object from createContext with a inject and a provide property. This will move all property access on the return of the createContext function into actual component setup functions, and will therefore be able to be properly tree shaken. This is a bigger task and would also be backwards incompatible since createContext is exported. I did some tests in https://github.com/jsmnbom/radix-vue/tree/improve-treeshaking and the results do speak for themselves:
Note that rollup currently does not seem to think that spread statements or destructures can have side effects (rollup/rollup#3984), but should that ever get fixed then this issue will show up with rollup builds too.
Describe the feature
Currently when using radix-vue in a project that is built using esbuild tree shaking does not work very well due to various reasons. This can be easily seen by bundling a simple file like:
On my machine this will currently create a 75kB file, as seen in the below analysis:
npx esbuild --bundle --outfile=dist/simple.js --format=esm --minify --analyze simple.js
The reason for this huge size turns out to be due to evanw/esbuild#3392 as well as two single statements in https://github.com/radix-vue/radix-vue/blob/main/packages/radix-vue/src/Menu/MenuContentImpl.vue and https://github.com/radix-vue/radix-vue/blob/main/packages/radix-vue/src/Popper/PopperContent.vue.
The best workaround I have found is to move the destructure into the
<script>
inside a IIFE as suggested in the issue above.Example diff of putting default props inside a IIFE.
This will in turn lead to a 32kB file as the simple output from above.
npx esbuild --bundle --outfile=dist/simple.js --format=esm --minify --analyze simple.js
I can submit a PR with this change, but I chose to open an issue since I don't know if this is the way you would like to fix this issue. Alternatively the popper props can just be put right into the defineProps statement:
const props = withDefaults(defineProps<PopperContentProps>(), PopperContentPropsDefaultValue)
but that excludes the possibility of adding more props in the future toMenuContentImpl
that do not also apply toPopperContent
- tho maybe that's okay?Another treeshaking issue comes from the use of
createContext
. SincecreateContext
is used inside plain<script>
tags in .vue files;export const [injectPopperContentContext, providePopperContentContext] = createContext<PopperContentContext>('PopperContent')
. The statement will be added verbatim in the bundle (although minified), and since it is a destructure statement esbuild will not be able to treeshake it. I propose instead to export an object fromcreateContext
with ainject
and aprovide
property. This will move all property access on the return of thecreateContext
function into actual component setup functions, and will therefore be able to be properly tree shaken. This is a bigger task and would also be backwards incompatible sincecreateContext
is exported. I did some tests in https://github.com/jsmnbom/radix-vue/tree/improve-treeshaking and the results do speak for themselves:npx esbuild --bundle --outfile=dist/simple.js --format=esm --minify --analyze simple.js
Note that rollup currently does not seem to think that spread statements or destructures can have side effects (rollup/rollup#3984), but should that ever get fixed then this issue will show up with rollup builds too.
See https://github.com/jsmnbom/radix-vue/tree/improve-treeshaking for the changes I did, and do say if a PR is welcome with the changes, or if this should be handled differently :)
Additional information
The text was updated successfully, but these errors were encountered: