denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
97.74k stars 5.38k forks source link

`{deno:{namespace:false}}` worker option doesn't remove `Deno` namespace #26056

Open josephrocca opened 1 month ago

josephrocca commented 1 month ago

Version: Deno 1.46.x and 2.x

This logs Received from worker: Hello from Worker! 2.0.0-rc.10+053894b instead of throwing a reference error:

const workerBlob = new Blob([`
  self.onmessage = async function(e) {
    self.postMessage('Hello from Worker! ' + Deno.version.deno);
  }
`], { type: 'application/javascript' });

const workerURL = URL.createObjectURL(workerBlob);

const worker = new Worker(workerURL, {type:"module", deno:{permissions:"none", namespace:false}});

worker.onmessage = function(e) {
  console.log('Received from worker:', e.data);
};
worker.postMessage('Hello from main thread!');

This issue may be more of a feature request, since it's not explicitly mentioned in the docs that namespace:false disables Deno stuff. It only says:

Starting in v1.22 the Deno namespace is available in worker scope by default. To enable the namespace in earlier versions pass deno: { namespace: true } when creating a new worker.

Note if delete globalThis.Deno is equivalent, this may not be an ideal solution for me, because part of the reason I want to remove the Deno stuff is to reduce the memory overhead of the worker.

lucacasonato commented 1 month ago

Yeah, this is not a supported feature. You should run delete globalThis.Deno. Memory overhead of the worker would not be different if we implemented this using namespace: false.

josephrocca commented 1 month ago

Yeah, this is not a supported feature.

Ah okay, no worries. I think it should be supported (for clarity/consistency, at least), and since satyarohith has labelled this issue as a suggestion, I'll leave it open, but of course feel free to close this if there's no intention/desire to support false as a valid value, since the workaround is easy anyway.