Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

[Feature] support for ignoring modules #95

Closed
symbyte opened this issue Oct 11, 2017 · 14 comments · Fixed by #264
Closed

[Feature] support for ignoring modules #95

symbyte opened this issue Oct 11, 2017 · 14 comments · Fixed by #264

Comments

@symbyte
Copy link

symbyte commented Oct 11, 2017

We are using the following function in order to keep certain modules (generally platform specific dependancies like sqlite3) from being bundled into our webpack bundle which will cause any module in the array to be manually required instead:

  externals: [
    (function () {
      var IGNORES = [
        'buffer',
        'fs',
        'electron',
        'sqlite3',
        'JSONStream',
        'request',
        'zlib',
        'bindings',
        'leveldown',
        'pouchdb',
        'canvas'
      ];
      return function (context, request, callback) {
        if (IGNORES.indexOf(request) >= 0) {
          return callback(null, "require('" + request + "')");
        }
        return callback();
      };
    })()
  ]

When loading a worker that uses any of those modules directly or uses a module that uses them, webpack attempts to trace back these require statements instead of ignoring them as the above config dictates leading to build errors like:

    ERROR in ./~/sqlite3/~/node-pre-gyp/lib/info.js
    Module not found: Error: Can't resolve 'aws-sdk' in '/Users/symbyte/projects/my-project/node_modules/sqlite3/node_modules/node-pre-gyp/lib'
     @ ./~/sqlite3/~/node-pre-gyp/lib/info.js 14:14-32
     @ ./~/sqlite3/~/node-pre-gyp/lib ^\.\/.*$
     @ ./~/sqlite3/~/node-pre-gyp/lib/node-pre-gyp.js
     @ ./~/sqlite3/lib/sqlite3.js
     @ ./~/ts-loader?{"useTranspileModule":true}!./~/tslint-loader?{"configFile":"/Users/symbyte/projects/my-project/tslint.json"}!./~/tslint-loader!./src/app/data/workers/geojson-io.worker.ts

Is there some way to get worker-loader to acknowledge this part of my webpack config, have I done something incorrectly, or is this a bug?

@michael-ciniawsky michael-ciniawsky changed the title webpack option "externals" not used when loading worker [Feature] support for ignoring modules Oct 12, 2017
@michael-ciniawsky michael-ciniawsky added this to the 1.1.0 milestone Oct 12, 2017
@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Oct 24, 2017

Could you try changing this line (L61) to

- .createChildCompiler('worker', worker.options);
+ .createChildCompiler('worker', Object.assign({}, worker.options, this.options.externals));

and try if that works ? 🙃

@symbyte
Copy link
Author

symbyte commented Oct 24, 2017

Sadly it does not. Still have the above reported errors 😢

@michael-ciniawsky
Copy link
Member

hmm... ok then I need to investigate this further 🙃

@michael-ciniawsky
Copy link
Member

Would it be possible to spin a repo for reproduction ?

@symbyte
Copy link
Author

symbyte commented Oct 24, 2017

It may take me a several days, but I'll try to put something together!

@symbyte
Copy link
Author

symbyte commented Oct 29, 2017

Before creating a repo for reproduction, I wanted to do some investigation myself to see if I could find a way to add this (mostly because I've always wanted an excuse to learn more about how webpack loaders work). I learned a good bit, but I wanted to check some of my assumptions here and ask some questions.

One odd thing that I saw was that when using the IIFE shown above to create an externals function, the externals entry in this.options within the worker-loader was resolving to

externals: [
  null
],

Is this maybe a quirk in the translation of my webpack config to the options that are made available to a given loader?

I sidestepped this by specifying the externals entry instead as an object like so:

  externals: {
    'buffer': 'commonjs buffer',
    'fs': 'commonjs fs', 
    'electron': 'commonjs electron',
    'sqlite3': 'commonjs sqlite3',
    'JSONStream': 'commonjs JSONStream',
    'request': 'commonjs request',
    'zlib': 'commonjs zlib',
    'bindings': 'commonjs bindings',
    'leveldown': 'commonjs leveldown',
    'pouchdb': 'commonjs pouchdb',
    'canvas': 'commonjs canvas',
  }

Which allowed the externals entry to be correctly represented in this.options within the worker-loader.

I then retried the above suggested fix, thinking that the null was maybe the cause for it not working, but I still got the same result. Upon inspection the config for the created child compiler came out as:

{
  "output": {
    "filename": "[hash].worker.js",
    "chunkFilename": "[id].[hash].worker.js",
    "namedChunkFilename": null
    "externals": {
      "buffer": "commonjs buffer",
      "fs": "commonjs fs",
      "electron": "commonjs electron",
      "sqlite3": "commonjs sqlite3",
      "JSONStream": "commonjs JSONStream",
      "request": "commonjs request",
      "zlib": "commonjs zlib",
      "bindings": "commonjs bindings",
      "leveldown": "commonjs leveldown",
      "pouchdb": "commonjs pouchdb",
      "canvas": "commonjs canvas"
    }
  }
}

This seems a bit odd to me, because I assume that the child compiler options would be a similar structure to the root compiler options (with externals being a peer to output). I altered the above suggested fix to give an options structure that fit these expectations:

  worker.compiler = this._compilation
    .createChildCompiler('worker', worker.options);

  worker.compiler.options.externals = this.options.externals;

which yielded the structure:

{
  "output": {
    "filename": "[hash].worker.js",
    "chunkFilename": "[id].[hash].worker.js",
    "namedChunkFilename": null
  },
  "externals": {
    "buffer": "commonjs buffer",
    "fs": "commonjs fs",
    "electron": "commonjs electron",
    "sqlite3": "commonjs sqlite3",
    "JSONStream": "commonjs JSONStream",
    "request": "commonjs request",
    "zlib": "commonjs zlib",
    "bindings": "commonjs bindings",
    "leveldown": "commonjs leveldown",
    "pouchdb": "commonjs pouchdb",
    "canvas": "commonjs canvas"
  }
}

But even doing this, the result was still the same.

This got me thinking about the child compiler: given that by default (no matter what options are passed to it in the createChildCompiler call) it only creates an output entry in the config for the resulting compiler, does a child compiler only support output as a configuration option? I wasn't able to find any real documentation about child compilers, but it would make sense that this would be the case given that it also has a reference to its parent compiler that has all the other options configured.

Still, I'd think if this were the case then the externals entry with the modifications I've detailed here would be taken into account, but alas they still are not.

This makes me think that this is either:

  1. This is a limitation of using a child compiler (it is intended that externals is not taken into account and that the child compiler has no knowledge of its parent compilers options)
  2. It is a bug with a child compiler not correctly referencing the options of its parent.

Am I thinking about this correctly? I've made a lot of assumptions that I'm just not sure about based on my console.log driven inspection of the this._compilation member in the pitch function.

@symbyte
Copy link
Author

symbyte commented Oct 29, 2017

Just threw together a minimal reproduction repo here: https://github.com/symbyte/worker-loader-externals

@symbyte
Copy link
Author

symbyte commented Nov 7, 2017

@michael-ciniawsky Do you have any ideas on the stuff I mentioned above?

@corytheboyd
Copy link

Just ran into a case where this would be very helpful. I am integrating a Worker into the background of a Chrome Extension, and am using a HMR library with a dependency on window.chrome, which is obviously absent from the Worker sandbox.

@timkendrick
Copy link

I was hitting this exact same problem in a custom loader I was writing. I think it's due to the fact that although the child compiler does inherit all its parent's options, it appears that not all of the parent's plugins are inherited.

From digging around a bit, it looks in a normal compilation the externals option is actually shorthand for applying an ExternalsPlugin, as seen in WebpackOptionsApply.js. This happens automatically when using the top-level webpack() function but I don't think the options are processed in the same way when creating a child compiler via createChildCompiler().

I've worked around this in my loader by manually adding the ExternalsPlugin to the child compiler options:

if (worker.compiler.options.externals) {
  worker.compiler.apply(new ExternalsPlugin(
    worker.compiler.options.output.libraryTarget,
    worker.compiler.options.externals
  ));
}

…this seems to fix the problem for my use case.

@michael-ciniawsky it looks like there's quite a lot of other stuff in the WebpackOptionsApply.js that isn't being applied to the child compiler – do any of these also need to be applied to the child compilation, or are they mostly unnecessary?

@nschwan94
Copy link

@symbyte I appreciate you filing this issue. I'm in a position where I need to use the 'sqlite3' module in a Web Worker script. I followed along the posts of you and timkendrick pretty well, but wasn't able to get anything functional. Has there been any workaround established in the meanwhile? I'm having trouble understanding exactly the root of this issue, but I thought this would be feasible.

@timkendrick
Copy link

@nschwan94 I might well be wrong, but I suspect sqlite relies on compiled native modules and therefore won't be able to be run outside a Node.js environment. If that's the case you won't be able to run it in the browser at all, never mind in a web worker…

@nschwan94
Copy link

@timkendrick thanks for the reply. I'm attempting this in an Electron app. The thought was to launch a few Web Workers that could each read/write from/to separate sqlite databases.

@tlaukkan
Copy link

Hi, any chance of getting this fixed. I think this is a bug really.

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

Successfully merging a pull request may close this issue.

6 participants