parcel-bundler / parcel

The zero configuration build tool for the web. ๐Ÿ“ฆ๐Ÿš€
https://parceljs.org
MIT License
43.43k stars 2.26k forks source link

Loading script to AudioWorklet #1093

Closed ronkot closed 3 years ago

ronkot commented 6 years ago

โ“Question:

I'm trying to use new AudioWorklet feature (enabled by default in Chrome 66 ->). Part of the process is to load a WorkletProcessorScript to a web worker from my main js file like this:

...
audioContext.audioWorklet.addModule('my-worklet-processor.js').then(() => {
  ...
});

So I'd need to keep my-worklet-processor.js as a separate file. Maybe this is already possible, but I can't find a way to do this. Any clues how to solve this? Big thanks! ๐Ÿ™

There's not yet good documentation on AudioWorklets, but here and here are some examples I've been using.

๐ŸŒ Your Environment

Software Version(s)
Parcel 1.4.1
Node 8.9.4
npm/Yarn npm 5.7.1
Operating System OSX 10.11.6
mischnic commented 6 years ago

Maybe it needs to be added to https://github.com/parcel-bundler/parcel/blob/0140dcea58edd0df5959752e38faf41b96a1d509/src/assets/JSAsset.js#L19 to be treated like a web worker.

mischnic commented 6 years ago

@ronkot What happens currently? Does 'my-worklet-processor.js' get replaced with something else?

ronkot commented 6 years ago

@mischnic my-worklet-processor.js does not seem to get replaced (if I understood your question correctly). File is requested as http://localhost:1234/js/my-worklet-processor.js (running in dev mode), which in turn returns the content of my index.html.

mischnic commented 6 years ago

This correctly detects the calls, but the emitted worklet file contains the hmr runtime.

diff --git a/src/assets/JSAsset.js b/src/assets/JSAsset.js
index 7261c35..6fffbb5 100644
--- a/src/assets/JSAsset.js
+++ b/src/assets/JSAsset.js
@@ -16,7 +16,7 @@ const IMPORT_RE = /\b(?:import\b|export\b|require\s*\()/;
 const GLOBAL_RE = /\b(?:process|__dirname|__filename|global|Buffer)\b/;
 const FS_RE = /\breadFileSync\b/;
 const SW_RE = /\bnavigator\s*\.\s*serviceWorker\s*\.\s*register\s*\(/;
-const WORKER_RE = /\bnew\s*Worker\s*\(/;
+const WORKER_RE = /\bnew\s*Worker\s*\(|audioWorklet\.addModule\s*\(/;

 class JSAsset extends Asset {
   constructor(name, pkg, options) {
diff --git a/src/visitors/dependencies.js b/src/visitors/dependencies.js
index e697db7..4652f81 100644
--- a/src/visitors/dependencies.js
+++ b/src/visitors/dependencies.js
@@ -61,6 +61,13 @@ module.exports = {
       return;
     }

+    if(types.isStringLiteral(args[0]) && types.isMemberExpression(callee) &&
+       callee.object.property && callee.object.property.name === "audioWorklet" &&
+       callee.property.name === "addModule") {
+      addURLDependency(asset, args[0]);
+      return;
+    }
+
     const isRegisterServiceWorker =
       types.isStringLiteral(args[0]) &&
       matchesPattern(callee, serviceWorkerPattern);
ronkot commented 6 years ago

Any news on this? Could I help somehow - by providing a minimal working example for example?

Also, I'd be happy to hear if there's some temporary hack that I could use.

habemus-papadum commented 6 years ago

I would be interested in some basic guidance about this as well.

My thoughts are for now to inline the script as multiline string, convert it to a Blob, and use the Blob's url in addModule.

This looses all the potential transpilation wins parcel provides but since these scripts are meant to be lean and mean, they probably need a tailored transpilation approach anyway (ala glslify, perhaps)

habemus-papadum commented 5 years ago

@mischnic Using your patches suggested above, does the hmr runtime added to the Audio Worklet script cause to script to fail, or is the hmr code functional in the AudioWorklet context (which doesn't seem all that bad...) ? (Or has this not been tested -- in which case I can try)

mischnic commented 5 years ago

does the hmr runtime added to the Audio Worklet script cause to script to fail

I don't recall the exact error.

is the hmr code functional in the AudioWorklet context

HMR won't work because you can register a AudioWorkletProcessor only once:

If the name exists as a key in the node name to processor definition map, throw a NotSupportedError exception and abort these steps because registering a definition with a duplicated key is not allowed. (https://webaudio.github.io/web-audio-api/#dom-audioworkletglobalscope-registerprocessor)

habemus-papadum commented 5 years ago

@mischnic Thanks! all makes sense

quasicomputational commented 5 years ago

This'd be solved by #2306. If you want to try a hackish workaround today, you might try my parcel-plugin-import-as-url.

mischnic commented 5 years ago

This'd be solved by 2306

That's not needed for this, normal WebWorkers work fine with Parcel.

H-s-O commented 4 years ago

Any news on this ?

mischnic commented 3 years ago

Now, there are some more worklet types with CSS Houdini

ProLoser commented 3 years ago

@mischnic could either implicitly detect *worklet.addModule or could just let people deal with this themselves explicitly by doing some sort of import worklet from 'worklet:./some/file.js' and this way you don't have to deal with all the edge cases around the syntax.

mischnic commented 3 years ago

audioWorklet.addModule is very easy to add, it's essentially the same as new Worker at the moment. The only difference to webworkers is that worklets don't support importScripts, but only ESM. (and we'd want to only use the syntax here: https://github.com/parcel-bundler/parcel/issues/5430 for worklets and then deprecated the current new Worker("url") syntax)

A pipeline that returns a URL to a worker is harder to do....

ProLoser commented 3 years ago

@mischnic so if I understand things correctly, your suggested fix works as long as I use --no-hmr for serve and it shouldn't be an issue for bundle, correct? I already have HMR disabled due to worklet problems, this would be an acceptable compromise for me.

mischnic commented 3 years ago

Yes (so the approach from https://github.com/parcel-bundler/parcel/issues/1093#issuecomment-377218948 but for v2 and using https://github.com/parcel-bundler/parcel/issues/5430)

ProLoser commented 3 years ago

Does this look correct?

// @parcel/transformer-js/src/visitors/dependencies.js
      if (types.isStringLiteral(args[0]) && types.isMemberExpression(callee) &&
        callee.object.property && callee.object.property.name === "audioWorklet" &&
        callee.property.name === "addModule") {

        addURLDependency(asset, ast, args[0], {
          env: {
            context: 'worker',
            outputFormat:
              isModule && asset.env.scopeHoist ? 'esmodule' : undefined,
          },
          meta: {
            webworker: true,
          },
        });
        return;
      }

Pinging @dioptre

mischnic commented 3 years ago

I thought that audioWorklet was a global variable (like service worker), but apparently not. This makes detecting the call much more problematic...

ProLoser commented 3 years ago

Can we detect worklet.addmodule( (case-insensitive) or is it complicated due to the AST?

mischnic commented 3 years ago
  1. detect worklet.addmodule(?

That is not a problem, but how would you differentiate this from

class Foo {
    addModule(data) {
        ...
    }
}
let worklet = new Foo();
worklet.addModule(....)

(= it's not really possible to determine if worklet is actually an AudioContext)

  1. just set isIsolated for these imports in a transformer?

This breaks once you there are shared bundles in the worker, because asset.env.context isn't set to "worker" or something like that.

I also found that adding an import to the worklet results in a worklet.js doesn't export 'default' error.

ProLoser commented 3 years ago

Yes I've seen some other bundler wrap the contents in an export default {code} but wasn't sure why.

Personally, I wish this was a little bit more declarative and explicit, I really like the xyz: pattern as it's clear even if you never seen it before. I believe we should add a worker: transformer that applies the context and bundling correctly. Would that fix things? Do worklets just need to have a default export vs workers?

As long as we had SOME solution for now to be unblocked that is flexible for everyone's weird scenario would be great, we can try to get the automagic working later (and possibly integrate with native apis better if supported)

ProLoser commented 3 years ago

Would that be because the import is not treated as a runtime entrypoint so it's expected to have some sort of export?

ProLoser commented 3 years ago

I think detecting worker.addModule() is totally acceptable: http://developer.mozilla.org/en-US/docs/Web/API/Worklet/addModule

Although I guess some weird obscure library dependency could break :(

I would clearly document it prominently and see who complains lol.

ProLoser commented 3 years ago

I'm testing this out:

// @parcel/transformer-js/src/visitors/dependencies.js
// ...
CallExpression: {
// ...
    let isWorkletAddModule =
        types.isMemberExpression(callee) &&
        types.matchesPattern(callee, 'Worklet.addModule') && // (W
        args.length >= 1 &&
        types.isStringLiteral(args[0]) &&
        !isInFalsyBranch(ancestors);

      if (isWorkletAddModule) {
        let isModule = false;
        if (types.isObjectExpression(args[1])) {
          let prop = args[1].properties.find(v =>
            types.isIdentifier(v.key, { name: 'type' }),
          );
          if (prop && types.isStringLiteral(prop.value))
            isModule = prop.value.value === 'module';
        }

        addURLDependency(asset, ast, args[0], {
          env: {
            context: 'web-worker',
            outputFormat:
              isModule && asset.env.shouldScopeHoist ? 'esmodule' : undefined,
          },
          meta: {
            webworker: true,
          },
        });
        return;
      }
// ...
mischnic commented 3 years ago

As I've said, your version of detecting worker.addModule() might work in your case. But it's not safe generally and we certainly can't do this for everyone with the default config unless worker is proven to be an actual native AudioWorklet object.

I've pushed an experiment from some time ago for supporting worker: and worklet: imports: https://github.com/parcel-bundler/parcel/tree/worker-url. The current implementation would certainly break once shared modules between worklets and other contexts are introduced.

ProLoser commented 3 years ago

Oh that's awesome! I understand that reservation. I will play with it, luckily my worklet code is not complex enough to have any dependencies (yet)

ProLoser commented 3 years ago

Okay I've checked out your branch, reinstalled xcode, linked everything, went into my project and linked in the packages you modified in your worker-url branch (core, transformer-worker/worklet, config-default) and built my project. Unfortunately I'm seeing this error:

 ~/S/zeus ๎‚ฐ ๎‚  master ยฑ ๎‚ฐ yarn browser:build --no-cache
yarn run v1.22.10
$ yarn workspace browser build --no-cache
$ parcel build ./src/index.html --no-cache
๐Ÿšจ Build failed.
@parcel/packager-js: esmodule output is not supported without scope hoisting.
Error: esmodule output is not supported without scope hoisting.
    at Object.package (/Users/dsofer/Sites/parcel/packages/packagers/js/lib/JSPackager.js:187:13)
    at PackagerRunner.package (/Users/dsofer/Sites/parcel/packages/core/core/lib/PackagerRunner.js:350:27)
    at async PackagerRunner.getBundleResult (/Users/dsofer/Sites/parcel/packages/core/core/lib/PackagerRunner.js:311:20)
    at async PackagerRunner.getBundleInfo (/Users/dsofer/Sites/parcel/packages/core/core/lib/PackagerRunner.js:303:9)
    at async Child.handleRequest (/Users/dsofer/Sites/parcel/packages/core/workers/lib/child.js:255:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed.
Exit code: 1
Command: /Users/dsofer/.nvm/versions/node/v14.15.2/bin/node
Arguments: /usr/local/Cellar/yarn/1.22.10/libexec/lib/cli.js build --no-cache
Directory: /Users/dsofer/Sites/zeus/packages/browser
Output:

info Visit https://yarnpkg.com/en/docs/cli/workspace for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

In my code I have the following:

import encoderWorker from 'worklet:./encoderWorker.js';

// ...
audioWorklet.addModule(encoderWorker)
ProLoser commented 3 years ago

@mischnic do I need to link every single parcel package when using this fork? Or just a specific subset?

mischnic commented 3 years ago

The easiest way would probably be just copying the transformers into your project (if you're using a monorepo anyway), because there aren't any changes to Parcel core anyway.

ProLoser commented 3 years ago

I'm looking at your diff https://github.com/parcel-bundler/parcel/compare/worker-url it looks like you do modify the core but I'll try copying the packages over

ProLoser commented 3 years ago

It's kinda hard to understand why this is happening, such is the intricacies of a monorepo, any chance this branch could get merged? image

ProLoser commented 3 years ago

Parcel keeps trying to reinstall packages when I run it and it's clearing out the copied over packages. I don't really know of a quicker way to implement this other than getting it into a nightly release.

shaibam commented 3 years ago

I can't beleive I'm saying this but inlining it worked for me: https://stackoverflow.com/questions/55503925/can-the-paintworklet-be-inlined