Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Web Workers coming from node_modules #670

Closed
wojtekmaj opened this issue Jan 25, 2018 · 11 comments · Fixed by #6696
Closed

Support Web Workers coming from node_modules #670

wojtekmaj opened this issue Jan 25, 2018 · 11 comments · Fixed by #6696

Comments

@wojtekmaj
Copy link
Contributor

wojtekmaj commented Jan 25, 2018

Choose one: is this a 🐛 bug report or 🙋 feature request?

Both, I guess?

🤔 Expected Behavior

  • node_modules should be also checked when looking for a file

😯 Current Behavior

  • trying to refer to a Service Worker file from node_modules fails with the following error:
×  C:\Users\wojtekmaj\Projekty\react-pdf\sample\parcel\node_modules\react-pdf\build\entry.parcel.js: Cannot resolve dependency './pdfjs-dist\build\pdf.worker.js' (...)

what it is trying to do is look for the path in ./ but not in node_modules.

🔦 Context

💻 Code Sample

I'm currently working on support for Parcel in React-PDF. That requires me to handle loading a service worker which is not mine.

In Webpack, the following code does the job:

var pdfjs = require('react-pdf/build/pdf.js');
var PdfjsWorker = require('worker-loader!react-pdf/build/pdf.worker.js');

if (typeof window !== 'undefined' && 'Worker' in window) {
  pdfjs.PDFJS.workerPort = new PdfjsWorker();
}

I was trying to reproduce the same using Parcel, and here's what I got so far:

const pdfjs = require('pdfjs-dist');

if (typeof window !== 'undefined' && 'Worker' in window) {
  pdfjs.PDFJS.workerPort = new Worker('pdfjs-dist/build/pdf.worker.js');
}

This results in an error described below. Changing the line to

pdfjs.PDFJS.workerPort = new Worker('../node_modules/pdfjs-dist/build/pdf.worker.js');

stops the error, but:

  • I'm not comfortable with relative path in a package like that
  • This caused.. a 404 error! Parcel starts referring to a file it did not produced.

🌍 Your Environment

Software Version(s)
Parcel 1.5.0
Node 9.3.0
npm/Yarn npm, newest
Operating System Windows 10
@wojtekmaj
Copy link
Contributor Author

Here's my full code at the moment if you'd like to have a look:

https://github.com/wojtekmaj/react-pdf/tree/parcel/sample/parcel

@devongovett
Copy link
Member

Ah this is an interesting issue. We treat the argument to Worker as a URL, which it can be. That means this is being treated as a relative path rather than a Node module path. We could maybe require the path to start with ./ or ../ in order to be relative? Not sure we can change this everywhere URL paths are used though since relative URLs without that are fairly common.

@wojtekmaj
Copy link
Contributor Author

Actually that would be great. Not having ./ in front of the path suggests that the path either:

  • is a path to node_module
  • is a path set in jsconfig.json in compilerOptions.paths field as kind of a "root" directory

I'd greatly appreciate this improvement. I'm really looking forward to support Parcel :)

@mischnic
Copy link
Member

The correct term would be web worker. Service workers handle caching and push notifications.

@wojtekmaj wojtekmaj changed the title Support Service Workers coming from node_modules Support Web Workers coming from node_modules Jan 27, 2018
wojtekmaj added a commit to wojtekmaj/react-pdf that referenced this issue Jan 28, 2018
@wojtekmaj
Copy link
Contributor Author

Guys, is there any news regarding that matter?

I'd really like to officially support Parcel in React-PDF, but I'm super unsure about web worker necessary for my project to run correctly. It seems like my workaround is doing the trick, but to me it looks just messy. Is there an official, recommended way to handle this?

@mischnic
Copy link
Member

mischnic commented Feb 25, 2018

@devongovett Inspired by the new resolver:

  • "~/node_modules/pdfjs-dist/build/pdf.worker.js" for "[../]node_modules/pdfjs-dist/build/pdf.worker.js"
  • "lib/worker.js" = "./lib/worker.js" for relative paths

@devongovett
Copy link
Member

For Parcel 2, we are considering supporting an npm: URL protocol to resolve node_modules instead of as a relative path. For example:

new Worker('npm:pdfjs-worker');

Without the npm:, it would resolve the same as ./pdfjs-worker, since it is a URL.

See #3492 (comment) for more details.

@misha-erm
Copy link

Looks like I have some similar issue.
I'm trying to use a node_module which uses a worker-loader and get the error below.

image

Are there any workarounds? Really don't want to switch to webpack because of that issue(((
Thanks in advance

@devongovett
Copy link
Member

devongovett commented Nov 17, 2019

worker-loader is a webpack specific tool and won’t work with parcel. You can construct a worker like this:

new Worker(./worker.js”)

@mischnic
Copy link
Member

@MikeYermolayev I fear there's nothing you can do here. (And I think publishing code with worker-loader calls is bad for exactly this reason).

@misha-erm
Copy link

@mischnic yep. Had to reject using that library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@devongovett @mischnic @wojtekmaj @davidnagli @misha-erm and others