feat(worker): support worker format, plugins and rollupOptions (#6191)

[poyoho]

Description

fix: #6191

Additional context

shallow copy of the plugin config

---

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.
• 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.

[poyoho]

how can I update package.json in package/playground/worker 🙈

[poyoho]

when I use pnpm i will change this pnpm-lock.yaml with npm registry. will break the ci.

[patak-dev]

Check previous discussions in #5390 (comment), but a factory seems unnecessary after further discussion with others in the team.

For context, both in the PR and in #5390, users may include plugins that aren't intended to work in the bundling of workers (doing post-processing at build time for example). Plugins may also have cached that could be corrupted by being used in these extra rollup calls. If you check the current code, new internal Vite plugins are being recreated for each worker bundle. We discussed with the team and we think that the best we can do here is to let the user specify what plugins should be applied to workers, even if this means there will be some repetition in the config (but it could be easily avoided by extracting a common function).
We should have a new worker entry:

  worker: {
    // To tackle https://github.com/vitejs/vite/pull/5403
    format: 'es'
    // Vite plugins (so the `apply: 'pre'/'post'` scheme works)
    plugins: [ ... ],
    // So users can configure other options like we let them do in the main bundle
    rollupOptions: { ... }
  }

@poyoho let us know if you want to work in the plugins part or if you have doubts with this approach, you could use this PR to do it. We can add each option in a different PR if that is better suited.

[poyoho]

oh, you are right.
plugin will exec the lifecycle but now no exec lifecycle in vite, like generateBundle.
adding config to solve this problem seems to be a good choice.

[ydcjeff]

Some comments & should we also add docs at https://github.com/vitejs/vite/blob/main/docs/config/index.md with Worker options?

[ydcjeff]

@patak-dev I tested this PR and this fixes #6191 but seems not fix #5402. Also, PR title seems incorrect for new worker options feature.

[poyoho]

I seem this PR set preserveEntrySignatures: false and format: "es" and now export the worker option, so can resolve?

[patak-dev]

preserveEntrySignatures: false seems to be the correct value for this setting for workers: https://rollupjs.org/guide/en/#preserveentrysignatures

I think we can set it when the format is "es" @poyoho 👍🏼

[poyoho]

may I add #5402 in the description?

[kepta]

@kepta, would you help us test this PR? Also, why was preserveEntrySignatures needed in #5403?

As far as I understand it tells rollup not to optimize for the worker's entry point, since that file doesn't export anything.

I tried this patch out and it gives me the following error with the es option:

To preserve the export signature of the entry module "app/worker-setup/setup/expose-naukar-worker.worker.ts", an empty facade chunk was created.

As @patak-dev said above, we will need to move forward with defaulting to preserveEntrySignatures=false for es type.

[patak-dev]

@poyoho could you add preserveEntrySignatures: false when the format is es?

[poyoho]

oh, sorry I forget commit

[poyoho]

I accidentally deleted repo before. How should I link this PR to the new branch😭 @patak-dev

[patak-dev]

I accidentally deleted repo before. How should I link this PR to the new branch😭 @patak-dev

Maybe you could use gh. There is a code button next to the PR title that gives you the command to get this branch:

gh pr checkout 6243

[poyoho]

yes, I use this way to create the branch, but the branch from check out is out of sync with this pr

[patak-dev]

That's strange, if you start from a clean repo or delete your branch, that command should get you to the right spot. I'll add the option and commit here now 👍🏼

[poyoho]

It seems that this command (gh pr checkout 6243) will only check out a new branch

I still have a few PR don't know what to do 😕

[patak-dev]

I see what you mean now by deleting the branch... I think we just confused GH. Maybe create a new PR and link to this one just in case? We don't get suggestions now also. I sent the commit here https://github.com/vitejs/vite/tree/fix/worker-with-plugin

[patak-dev]

I think we should add preserveEntrySignatures: false all the time and not just when the format is es. I don't see why it preserving them is useful in other formats

[poyoho]

yes, I think so. Worker don't need to export anything.

I think worker.rollupOptions should be omit by preserveEntrySignatures ?

[patak-dev]

I think worker.rollupOptions should be omit by preserveEntrySignatures ?

Yes, good catch

[egasimus]

Status of this PR? How to help?

[patak-dev]

IIUC, @poyoho is going to create a new PR for this one. We will merge this for 2.8 beta