feat: prerender & analyse in worker rather than subprocess to support Deno

[fernandolguevara]

use workers instead of child_process

related:
denoland/deno#17248

@benmccann would you be able to review this PR?

[changeset-bot]

🦋 Changeset detected

Latest commit: 333e7ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

this LGTM! thank you. just had one small observation, inline.

I'm not sure I totally understand how this affects Deno though — it looks like child_process and worker_threads are both implemented. Is there some nuance I'm missing?

• https://deno.land/std@0.142.0/node/child_process.ts
• https://deno.land/std@0.142.0/node/worker_threads.ts

[benmccann]

I'm not sure I totally understand how this affects Deno though — it looks like child_process and worker_threads are both implemented. Is there some nuance I'm missing?

It sounds to me like Deno implements child_process, but not communication between child and parent process and for that recommend using workers - or at least that's what I got from this article. @bartlomieju would you be able to confirm if that's correct?

[bartlomieju]

Thanks for the ping @benmccann.

It sounds to me like Deno implements child_process, but not communication between child and parent process

That's true, we currently don't have support for the child_process.send() API. Unfortunately our polyfill for the worker_threads module is broken at the moment, but given that you have a need for it, we can prioritize fixing it and get it done next week. Does that sounds reasonable to you?

[benmccann]

Thanks! Yeah, we can wait to merge this on our side then. I'm also fine going either the worker_threads or child_process.send() route if one is easier than the other

[bartlomieju]

Workers would definitely be easier - we just need to update once-working code. I'll prioritize this work over the next week and get back to you once it's released (I'll target to release in Deno v1.33.4 next week).

[dominikg]

iirc we have been using child_process to ensure resources are properly cleaned up (eg db connections). Is that still the case with worker?

[bartlomieju]

iirc we have been using child_process to ensure resources are properly cleaned up (eg db connections). Is that still the case with worker?

@dominikg I believe so - workers use separate OS threads, so once the worked is closed/terminated the resources will be released back to the OS.

[Rich-Harris]

This code guarantees that everything is correctly shut down — if it wasn't, the tests would never complete:

kit/packages/kit/test/prerendering/basics/src/hooks.server.js

Lines 20 to 23 in 9f5642d

	// this code is here to make sure that we kill the process
	setInterval(() => {
	console.log('process is still alive');
	}, 5000);

So given that the tests are passing I'm confident that it works correctly.

[pboguslawski]

Please take a look at #9989 - seems that using workers require also env cleanup.