Deploying to npm or unpkg

[pkerpedjiev]

Quick question: do you have a suggestion for how to deploy a library using threads.js?

When building with webpack, two scripts are generated: mylib.min.js and 0.mylib.worker.min.js. If I deploy them as an npm package, I can access https://unpkg.com/mylib@0.1.0/dist/mylib.min.js and https://unpkg.com/mylib@0.1.0/dist/0.mylib.worker.min.js.

But when I create a test script and include the main script:

<script src="https://unpkg.com/mylib@0.1.0/dist/mylib.min.js"></script>

It will try to load the worker from /0.mylib.worker.min.js and not from its location on unpkg. Is there a way to tell it to look for the worker in the same directory as the main script or do I need to hack this in using one of the techniques described on stackoverflow?

Edit: Final solution

Add an option _baseURL (string) to the worker options. It's only picked up by web workers (not in node.js) and let's you prefix the URL with some static base URL.

new Worker("./my-worker", {
  _baseURL: "https://unpkg.com/mylibrary@v1.2.3/"
})

[andywer]

Hey @pkerpedjiev, that's a pretty good question!

Have you tried to pass a fully-qualified URL to the worker constructor? I imagine something like this:

spawn(new Worker(process.env.NODE_ENV === "production" ? "https://unpkg.com/mylib@0.1.0/dist/0.mylib.worker.min.js" : "./path/to/worker"))

Might require a change in the threads-plugin, though.

[pkerpedjiev]

Hey, thanks for the quick reply! I haven't tried anything beyond what's in the original question. I don't think process.env.NODE_ENV will be defined when the script is being served through a regular server.

An alternative I'm considering is adding a sed step in npm run build that replaces ./mylib.worker.js with getScriptUrl().

[andywer]

Ahh, sorry, I didn't share that thought:

I suppose you build the library with webpack and publish the build files only. You can set the NODE_ENV to production in your library's webpack config.

On a second thought, maybe you don't want to webpack-build the package... Do you run webpack and publish the build files or do you intend to ship the source files as a library and let the application that uses your library build it with webpack?

[pkerpedjiev]

Yes, I'm using webpack and would build this as a package and publish it. For the time being I don't expect people to use it as an import in a larger application.

I was just confused because the process.env.NODE_ENV is in the library source code (rather than webpack.config) so I figured it would be evaluated at runtime rather than build time.

[andywer]

To be clear: The NODE_ENV check would be in the library source code, but if you build the package with webpack, then you can just use the DefinePlugin to statically replace NODE_ENV with production when building your package, so that you can run tests locally in your package and it will spawn the latest local worker, not the one from unpkg, but when built it will always spawn the unpkg worker.

That was my idea, at least 😉

[pkerpedjiev]

Just tried out your suggestion. I got the following error:

ReferenceError: falseundefined__webpack__worker__25 is not defined

[pkerpedjiev]

Just to clarify, I get the error above when trying to run using the dev server:

Plugin track pileup failed to instantiate. ReferenceError: falseundefined__webpack__worker__1 is not defined
    at new BAMDataFetcher (bam-fetcher.js:64)
    ...

The production build works fine. Here's is the relevant line in the output js file:

    this.worker = Object(dist["spawn"])(new dist["Worker"]( true ? getThisScriptLocation() + '/0.higlass-pileup.min.worker.js' : undefined));

getThisScriptLocation() is a function I added that returns base path where the calling script lives. I looked at threads-plugin but wasn't entirely sure where to even begin.

[andywer]

That's a bit strange. When I try, my production build fails already, but with a different error.

Code: 81e6410 (feature/support-remote-url-worker)
Error: https://travis-ci.org/andywer/threads.js/jobs/582091315

[pkerpedjiev]

Oh, you know what? You're right. It worked for me because there was already a worker script in the dist directory from a previous build. When I clear it out and run npm run build a new one doesn't get generated with the ... process.env.NODE_ENV === "production" ... line.

[andywer]

Also on a second thought, it's not wise to have the ternary operator inside the new Worker() as it's not statically analyzable and will throw the threads-plugin off its course.

Instead it should rather be something like this to keep it analyzable:

- spawn(new Worker(process.env.NODE_ENV === "production" ? "https://unpkg.com/mylib@0.1.0/dist/0.mylib.worker.min.js" : "./path/to/worker"))
+ spawn(process.env.NODE_ENV === "production" ? new Worker("https://unpkg.com/mylib@0.1.0/dist/0.mylib.worker.min.js") : new Worker("./path/to/worker"))

[pkerpedjiev]

I guess you already know this but that works in develop mode but not in production.

Perhaps there can be an option to Worker that can take a function which returns prefix for where the worker will be loaded from? Because the current proposal assumes that we know that the generated script will be 0...worker.min.js. I presume that if there's more than one worker it will be 1...worker.min.js and 2., etc..

[andywer]

I did some stuff 😉

Can you try again with threads@1.0.0-beta.5-feature-156 and threads-plugin@1.2.0?

[pkerpedjiev]

Cool! I installed those new packages but If I do what you suggested earlier:

    this.worker = spawn(
      process.env.NODE_ENV === 'production' ?
        new Worker(
          `${getThisScriptLocation()}/0.higlass-pileup.worker.min.js`,
        ) : new Worker('./bam-fetcher-worker.js'),
    );

Then running npm run build doesn't generate dist/0.higlass-pileup-worker.min.js. Also, ideally I wouldn't want to add 0.higlass-pileup.worker.min.js inside my code since that filename is automatically generated by threads.js. Does that make sense?

I don't know how feasible it is but a potential API could look like the following. threads-plugin would replace the './bam-fetcher-worker.js' with the generated worker file (e.g. '0.higlass-pileup-worker.min.js').

    this.worker = spawn(
          new ThreadsWorker('./bam-fetcher-worker.js', getThisScriptLocation())
    );

Or maybe that's already possible and I'm not calling the API correctly?

[andywer]

I think the issue is that the first new expression is again not statically analyzable. Pass a string literal like new Worker("https://unpkg.com/...") and it should work. Not sure if referencing an env var in the expression or an import from package.json, for instance, would still work.

Got to think about the API change...

[pkerpedjiev]

Hmmm, I don't know about hardcoding an unpkg location directly in the source. That seems like it introduces the opposite problem. Instead of being tied to /script.worker.js it's now tied to unpkg.com/.../0.script.worker.js with the downside that we now need to know the output name of the worker script (e.g. 0...worker...).

I appreciate your help and sorry for being so particular.

[pkerpedjiev]

Just tried my hand at implementing this functionality: #161

Seems to work for my use case:

    this.worker = spawn(
      new Worker('./bam-fetcher-worker.js',
        {},
        () => getThisScriptLocation()),
    );

[andywer]

I see your point and appreciate your dedication, we just need a different API change. Adding a new parameter to the constructor for one edge-case that only makes real sense in the browser is a bit much.

I quickly extended the worker options (2nd argument to constructor) by a new property _baseURL in the feature branch mentioned before.

Usage looks like this:

new Worker("./path/to/worker", { _baseURL: "https://unpkg.com/..." })

Published as threads@1.0.0-beta.5-feature-156.1. Can you try that, too, and tell me what you think about this approach?

(In case you wonder why that option is prefixed with _: If the W3C should one day extend the spec with new options we want them not to name-clash with our custom options)

[pkerpedjiev]

That looks great! I just did a quick check and it looks like it works with the caveat that _baseURL needs to end in a slash.

I'll test it more extensively this evening. Thanks for your help!!

[andywer]

Nice! Are you sure about the trailing slash, though? Since it uses the URL constructor it should work either way.

[pkerpedjiev]

Yup, this works and finds the script at http://localhost/dist/0.script.js:

    this.worker = spawn(
      new Worker('./bam-fetcher-worker.js', {
        _baseURL: `${thisScriptLocation}/`,
      }),
    );

This does not work. It looks for the script at http://localhost/0.script.worker.js when it's actually at http://localhost/dist/0.script.worker.js.

    this.worker = spawn(
      new Worker('./bam-fetcher-worker.js', {
        _baseURL: `${thisScriptLocation}`,
      }),
    );

thisScriptLocation contains the location of the executing script with no trailing slash.

[andywer]

Ahh, interesting. It's path override vs override host, but keep path. Yet another edge case... 🙈😅

I am happy you could figure it out, though! Shall I merge the PR and release or is there anything left to do?

[pkerpedjiev]

I think it looks good. Thanks for all your help!

[andywer]

Feature published as threads@1.0.0-beta.6 🚀

Thanks for being such a help getting that feature straight, too!