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.

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

feat(plugin-legacy): add option to output only legacy builds #9458

Closed
wants to merge 4 commits into from

Conversation

amirkian007
Copy link
Contributor

Description

fix #9050

Additional context

i tried to remove the modern chunks before the transformIndexHtml happens but ended up breaking the generated html file .


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • [X ] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added plugin: legacy enhancement New feature or request labels Aug 1, 2022
@sapphi-red
Copy link
Member

sapphi-red commented Aug 1, 2022

Thanks for the PR!

i tried to remove the modern chunks before the transformIndexHtml happens but ended up breaking the generated html file .

Would you expand on this?

BTW lint is failing because of a different reason. Merging main branch will fix this.

@amirkian007
Copy link
Contributor Author

Thanks for the PR!

i tried to remove the modern chunks before the transformIndexHtml happens but ended up breaking the generated html file .

Would you expand on this?

BTW lint is failing because of a different reason. Merging main branch will fix this.

reproduction
so basically the logical method is to remove any modern chunk before its entry(<script/>) is added to html. i tried some solutions like adding a new plugin with {enforce :pre} to delete chunks ,or during another plugin like legacyGenerateBundlePlugin, but somehow this will result in a html file without any <script/> or <link /> (for css).not sure if it's a bug or not ,i provided a reproduction for you. i believe the safest method is probably the one i provided in this PR, what's your opinion?... Thanks.

@sapphi-red
Copy link
Member

i provided a reproduction for you

It seems this was not working because transformIndexHtml is called only with legacy chunk while the original hook expects to be called with legacy and modern chunk.

what's your opinion?

Maybe we could completely skip modern chunk generation by removing modern chunk from rollupOptions.output?

@zhangchenna
Copy link

after using import.meta error

@amirkian007
Copy link
Contributor Author

amirkian007 commented Aug 6, 2022

Maybe we could completely skip modern chunk generation by removing modern chunk from rollupOptions.output?

@sapphi-red We could manipulate output options using outputOptions() hook, but i don't think that works because options like output.chunkFileNames and output.entryFileNames have default values and i couldn't find any options that would skip modern chunk creation. same thing for rollupOptions.input

@sapphi-red
Copy link
Member

We are appending the legacy output option here. I wonder if it is possible to replace rollupOptions.output here instead of using outputOptions hook.

const createLegacyOutput = (
options: OutputOptions = {}
): OutputOptions => {
return {
...options,
format: 'system',
entryFileNames: getLegacyOutputFileName(options.entryFileNames),
chunkFileNames: getLegacyOutputFileName(options.chunkFileNames)
}
}
const { rollupOptions } = config.build
const { output } = rollupOptions
if (Array.isArray(output)) {
rollupOptions.output = [...output.map(createLegacyOutput), ...output]
} else {
rollupOptions.output = [createLegacyOutput(output), output || {}]
}
},

@amirkian007
Copy link
Contributor Author

amirkian007 commented Aug 6, 2022

@sapphi-red I tested it and it is not possible to replace rollupOptions.output(the one that is for modern chunks => [name].[hash].js) from there...

@sapphi-red
Copy link
Member

Closing as #10139 implements in a more performant way.

@sapphi-red sapphi-red closed this Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin: legacy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@vitejs/plugin-legacy support only output legacy bundle
3 participants