feat(plugins/worker): support SharedWorker (resolve #2093)

[fi3ework]

Resolve #2093

Uses test.concurrent to test shared worker operation. It's still an experimental API. May there's a better way to do this test (or maybe add test for the compiled worker file instead of E2E test). 🤔

Update 1
use test.concurrent.each, this API is stable.

[fi3ework]

Don't know why appveyor failed. Seems like an unrelated failure.

[Shinigami92]

nitpick (non-blocking): I think the second optional chaining isn't needed, cause it's already checked in the first check

Suggested change

	parseWorkerRequest(id)?.sharedworker) != null
	parseWorkerRequest(id).sharedworker) != null

[fi3ework]

The first optional chaining checks worker property of qs parsed search query. We still need to check for sharedworker, right? 🤔

[Shinigami92]

(parseWorkerRequest(id)?.worker ?? parseWorkerRequest(id).sharedworker) != null
// Is the same as
(parseWorkerRequest(id)?.worker ?? parseWorkerRequest(id)?.sharedworker) != null

// I think it becomes a bit clearer this way
const temp = parseWorkerRequest(id)
(temp?.worker ?? temp.sharedworker) != null

[fi3ework]

Oh, you are right, I just made a mistake. I like your recommended style, but TS complains with single ?.. TS won't know if because temp is nullish or temp.worker is nullish so it requires write ?. again.

I think we had to use double ?. for type checking. Or checks parsedQuery in advance, but maybe a little verbose.

      if (
        isBuild &&
        (parsedQuery && (parsedQuery.worker ?? parsedQuery.sharedworker)) !=
          null
      ) {
        return ''
      }

[Shinigami92]

Ah yeah you are right (I was wrong I think)
Cause if parsedQuery would be undefined, then it falls into the second case and would be also undefined there

But I think your last example checking parsedQuery first could also be a solution

we loose short circuit here when calling the function before isBuild was checked
so maybe the best solution of both worlds would be to nest two ifs

if (isBuild) {
  const parsedQuery = parseWorkerRequest(id)
  if (parsedQuery && (parsedQuery.worker || parsedQuery.sharedWorker)) {
// maybe:  if (parsedQuery && (!!parsedQuery.worker || !!parsedQuery.sharedWorker)) {
    return ''
  }
}

[fi3ework]

Done

[fi3ework]

Made a little change.

parsedQuery.worker ?? parsedQuery.sharedworker

Should use ?? because the value will be '' if the property exists.

!= null

Use != null to distinguish between '' and undefined

[Shinigami92]

@fi3ework Could you please update your PR by merging the latest main and maybe fixing every conflicts and failing tests? 🙂
AppVeyor and CircleCI are removed and only GitHub Actions will run via latest main

[fi3ework]

@Shinigami92 Rebased. No conflict and test passed.

[fi3ework]

@patak-js Please take a look at this PR, maybe give some advice on the new test case. 😃

[patak-dev]

Thanks for splitting the options. We should document this feature here: https://vitejs.dev/guide/assets.html#importing-asset-as-string

[fi3ework]

@patak-js Appended ?sharedworker to the doc where ?worker appears.

[patak-dev]

Thanks for the work here @fi3ework. Evan added the contribution welcome to this one in the issue, so we can merge it once we get enough approvals 👍🏼

[antfu]

It seems like jest v27 breaks the test. Looking into it

[fi3ework]

@antfu CI should be fixed in #3583.