Skip to content

Check for deprecated rollupOptions.preserveModules #5

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

Closed
receter opened this issue Apr 26, 2023 · 7 comments · Fixed by #6
Closed

Check for deprecated rollupOptions.preserveModules #5

receter opened this issue Apr 26, 2023 · 7 comments · Fixed by #6
Assignees

Comments

@receter
Copy link
Contributor

receter commented Apr 26, 2023

When rollupOptions.output.preserveModules is true, the association between a chunk and the css it referenced will lose for some reason, in which case style injection will be skipped.

It is possible to bypass this security check if the deprecated option rollupOptions.preserveModules is used.

@emosheeep
Copy link
Owner

emosheeep commented Apr 26, 2023

So the problem is that you want to use preserveModules with this plugin?
This plugin depends on viteMetaData property injected by vite on each chunk file, I used to run with preserveModule, but as the tips saying, when it is enabled, i can't get the associations because viteMetaData is empty in renderChunk hook. I can get it in next hook generateChunk but if i modify file content, the sourcemap will break. I haven't found a good resolution yet

@receter
Copy link
Contributor Author

receter commented Apr 26, 2023

That would be great, but I read that this might be more complicated and definitely is another issue/feature request.

I just opened this because if it leads to unexpected behaviour, it might be good to show the warning in both cases. I can create a pull request if you want me to.

@emosheeep
Copy link
Owner

well, i got it, i'll appreciate if you could make a PR

@emosheeep
Copy link
Owner

emosheeep commented Apr 26, 2023

Oh I realized that I misunderstood the issue, i just thought you want to use preserveMoudle with this plugin😂

To split up the css I tried preserveModules with cssCodeSplit but this just leads to a bunch of unassignable css files (vitejs/vite#8057).

Now I know, what did you say is that you use preserveModules and cssCodeSplit independently, and you compared it with my plugin and choose the latter😄

@receter
Copy link
Contributor Author

receter commented Apr 26, 2023

#6

@receter
Copy link
Contributor Author

receter commented Apr 26, 2023

Yes.

But nevertheless, if your plugin would work with preserveModules it would be ideal for my use case.

I wonder if it is just a vite issue that viteMetaData is empty when preserveModules is true.

@emosheeep
Copy link
Owner

emosheeep commented Apr 26, 2023

I dived into the vite's implementation and found vite collect the associations among the chunks by itself,I don't think of it as a bug. Maybe we need to implement these logic by ourselves like vite. But for now I think preserveModules is not necessary for us, indeed, the users who use your library usually don't care about how your dist directory structure looks like.

@emosheeep emosheeep self-assigned this Apr 27, 2023
@emosheeep emosheeep linked a pull request Apr 27, 2023 that will close this issue
@emosheeep emosheeep added good first issue Good for newcomers and removed good first issue Good for newcomers labels May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants