fix(legacy): preserve async generator function invocation

[lsdsjy]

Description

This PR is a continuation of #14968, whose trick works perfectly fine on older versions of vite but fails on vite@>4.4.0 because of the version bump of esbuild to 0.18.2. esbuild@0.18.2 introduces pure annotations for iife expressions, so the annotated async generator function will be dropped (by minifiers I think).

Additional context

A shorter way to preserve the generator function here is:

(async function* g(){import.meta.url;import("_").catch(()=>1)})();

I don't know if that is enough for syntax detecting since the body won't be executed until .next() is called.

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, especially the Pull Request Guidelines.
• 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).
• Update the corresponding documentation if needed.
• Ideally, include relevant tests that fail without this PR but pass with it.

[sapphi-red]

We can skip .next() as the legacy browsers doesn't actually execute detectModernBrowserDetector but throws an error when parsing the code. Maybe we should add a comment that described that detectModernBrowserDetector doesn't have to be executed.

Would you add a test for this? I think we can check this by running [/import\s*(/, /import.meta/, /async\s*function\*/].every(r => r.test(code)) for the entry point and the polyfill chunk.

[lsdsjy]

I've added a test for modern chunk legacy guard.
Besides, I find that some playground tests for the legacy plugin are not working because of the incorrect match patterns. The modern chunks are like index-{hash}.js, which cannot match /index\./. I've updated them too.

[bluwy]

#14968 (comment) notes that updating the CSP hashes is a breaking change. I'm not sure if we exactly cover that in semver, but the right thing to do seems to be cutting another major?

Unfortunately I didn't test that that PR doesn't work in esbuild's minifier either. I'm not really sure what to do at the moment, but thought pointing it out here.

[patak-dev]

I think we can merge this one and release vite-plugin-legacy v6 with it. Even if v5 was released only a week ago, it seems the proper thing to do now.

[sapphi-red]

I don't think we should say changing the hash is a breaking change. Otherwise, we'll have to bump majors every time the inline script is changed. That will make it difficult to release a bug fix in minor/patches. I wonder if we should remove the actual value from the README and show the command to obtain the values instead and clarify the value might change in patches.

[sapphi-red]

@lsdsjy Would you remove the .next() part? (#15021 (comment))

Besides, I find that some playground tests for the legacy plugin are not working because of the incorrect match patterns. The modern chunks are like index-{hash}.js, which cannot match /index\./. I've updated them too.

Good catch! Thanks!

[lsdsjy]

@lsdsjy Would you remove the .next() part? (#15021 (comment))

@sapphi-red There're 2 ways to preserve the invocation of the async generator function:

1. call .next() on the returned AsyncGenerator object;
2. move the import.meta.url and import() statements into the async generator function (so that it's not an empty function anymore).

I'm taking the first approach here. I can switch to approach 2 but I'm not sure because I think the first one is clearer because these three parallel statements make it immediately apparent that they involve three language features(import.meta, dynamic import and async generator function).

[sapphi-red]

Ah, my bad, I was misunderstanding it.

[patak-dev]

I don't think we should say changing the hash is a breaking change. Otherwise, we'll have to bump majors every time the inline script is changed. That will make it difficult to release a bug fix in minor/patches. I wonder if we should remove the actual value from the README and show the command to obtain the values instead and clarify the value might change in patches.

It is technically a breaking change, as it will require a change to work for certain users. I think it isn't a problem that it is in the readme, but it isn't clear when users need to change these. I think it could be valid for us to say that we'll only release changes to these hashes in minors (if we want to avoid major releases), with a note in that section. Users with a strict policy could then set the range to the current minor and still get other patches.