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

bundling less 3.0.1 with web-pack, targeting web-worker. #3178

Closed
dasdeck opened this issue Feb 22, 2018 · 12 comments
Closed

bundling less 3.0.1 with web-pack, targeting web-worker. #3178

dasdeck opened this issue Feb 22, 2018 · 12 comments

Comments

@dasdeck
Copy link

dasdeck commented Feb 22, 2018

Hi!

I can not get less 3.0.1 to bundle properly into my web worker code.
Less 2 was working flawlessly with the same setup.

When I import less like this:

import less from 'less';

I get less.js:78 Uncaught ReferenceError: window is not defined in the browser console, which kind of makes sense, as the bundle uses the dist version for the browser but a worker has no window.

since the imports are static, I see no way of shimming the window away.

in the old version we did it like this:

import browser from 'less/browser'
const less = browser({},{});

which would set a window object.

This bundle, however, throws an error:

Cannot find module "."

which seems to be related to this particular line of code:
./browser/plugin-loader.js:9

var PluginLoader = function(less) {
    this.less = less;
    // shim for browser require?
    this.require = require;    <----------
};

here is my web-pack conf.

/* eslint-env node */

const conf = {

    entry: {
        'sidebar/test/app': './sidebar/test/app',
        'fields/test/app': './fields/test/app',
        'layout-editor/test/app': './layout-editor/test/app',
        'styler/test/app': './styler/test/app',
    },

    output: {
        filename: './[name].min.js'
    },

    externals: {
        vue: 'Vue',
        uikit: 'UIkit'
    },

    node: {
        fs: 'empty'
    },

    target: 'web',

    module: {

        rules: [
            {
                test: /\.vue$/,
                use: {
                    loader: 'vue-loader',
                    options: {
                        loaders: {
                            js: 'babel-loader?presets[]=env&plugins[]=lodash&plugins[]=transform-object-rest-spread'
                        }
                    }
                }
            },
            {
                test: /\.js$/,
                exclude: /node_modules/,
                use: {
                    loader: 'babel-loader',
                    options: {
                        presets: ['env'],
                        plugins: ['lodash', 'transform-object-rest-spread']
                    }
                }
            }
        ]
    }

}

module.exports = (env, argv) => ([conf, 
    {...conf, 
        entry: {'styler/lib/worker': './styler/lib/worker'}, 
        target: 'webworker',
    }]);

Cheers!

JM

@joepvl
Copy link

joepvl commented Apr 9, 2018

I'm using Less in a Web Worker as well, and also ran into issues with the new version. I'm not using any plugins or any of the browser functionality (automatically processing stylesheets in the DOM etc.) in my app, so for my case I was able to solve it as follows:

// in my worker
import Less from 'less/lib/less';
const less = Less();
less.PluginLoader = function() {};

less.render(/* your input */).then(result => console.log(result.css))

However, I was also running into Webpack build errors because of some inline require('promise') calls inside the less package. I didn't need or want promise as a dependency, so I ended up going with the following hack:

// in my webpack config
import ReplacePlugin from 'webpack-plugin-replace';

const config = {
  // ...
  plugins: [
    // ...
    new ReplacePlugin({
      exclude: /.*/,
      include: /node_modules\/less/,
      values: {
        "require('promise')": 'Promise'
      }
    }),
    // ...
  ]
  // ...
};

If you do actually need Less plugins and are directly or indirectly importing e.g. less/lib/less-browser/plugin-loader for that reason, you can expect Webpack to choke on the this.require = require line as well (or your browser might choke on the result), like what you're describing. Not sure if it's ever used though; you could try applying the same ReplacePlugin hack to that (for example replacing the line with '' or changing it into this.require = function() {}).

Not exactly pretty solutions, I know! 😅

@dasdeck
Copy link
Author

dasdeck commented Apr 9, 2018

Thanks for your help!
This did not help me yet, but it gets me some ideas for more things to try, so thanks for the input!
We need to use our plugin though, which might make it more difficult.

Cheers!

@joepvl
Copy link

joepvl commented Apr 10, 2018

My pleasure, but I have since found out, after some more poking and prodding, that I was basically completely wrong! My code/build "worked" for other reasons, and ReplacePlugin does not seem to do any work on the code before it reaches Webpack's processing/resolving logic (I don't know if this is even possible within Webpack), so importing less/browser will yield that same "Cannot find module" error because of the this.require = require line, in which the second require is replaced by some code generated by Webpack. In the end, I think this is basically because this code was not written with Webpack in mind. It seems like the browser-based require is actually (indirectly) passed to plugins in AbstractPluginLoader#evalPlugin (see abstract-plugin-loader.js#L66). This isn't compatible with Webpack's require mechanism, so it will complain:

WARNING in ./node_modules/less/lib/less-browser/plugin-loader.js
    12:19-26 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
     @ ./node_modules/less/lib/less-browser/plugin-loader.js
     @ ./node_modules/less/lib/less-browser/index.js
     @ ./node_modules/less/browser.js
     @ ./node_modules/babel-loader/lib!./app/lib/LessWorker.js

This isn't the type of dynamic require call you could fix using ContextReplacementPlugin as far as I can tell, since it is basically "too dynamic".

PS In addition to the above, I wrongly assumed that ReplacePlugin's include option would override exclude, so my setup actually did nothing because it excluded all paths. Yay, programming!

@joepvl
Copy link

joepvl commented Apr 10, 2018

TO ANYONE ELSE READING THIS: this is NOT a recommended way of doing things, especially in production!

You almost may as well fork Less.js to modify it for your own purposes at this point, which may actually be a better solution. I am only posting this for the sake of completeness. Hopefully we can find a way to officially support running Less in a web worker soon.


@dasdeck I was hacking around some more just now, and found a way to make it work, albeit in a very dirty way by most standards.

I even added the pi plugin from the docs, which seems to work as well (it loads, and calling pi() in the stylesheet will output 3.14159265), but it's only adding a very simple function so I don't know whether more complex plugins (e.g. visitors) would work.

Basically, it loads the precompiled version from less/dist/less.js into a raw string and evaluates that using the new Function method, injecting window and document shims that fulfil the bare minimum into the result. I'm guessing raw-importing the minified version would also work, btw. Here it is:

// using raw-loader here to import the file as a string
import LessRaw from 'raw-loader!less/dist/less';

const PiPlugin = {
  install: function(less, pluginManager, functions) {
    functions.add('pi', function() {
      return less.dimension(Math.PI);
    });
  }
};

const location = {
  href: '',
  protocol: 'https'
};

const document = {
  getElementsByTagName: () => [{ dataset: {} }],
  location
};

const window = {
  document,
  location,
  // less options that get picked up during Less initialization
  less: {
    onReady: false,
    plugins: [
      PiPlugin
    ]
  }
};

// put it all together
const Less = new Function('window', 'document', `${LessRaw}\n return window.less;`);
const less = Less(window, document);

// worker code
self.onmessage = function({ data }) {
  // `data` being Less code that you got from whatever posted the message to your worker
  less.render(data)
    .then(result => self.postMessage(result.css))
    .catch(error => console.warn('Error in worker', error));
};

@rjgotten
Copy link
Contributor

@joepvl
Use a specialized loader in your webpack config, chained to run before your worker-loader. Have that loader inject document and window shims as global variables within the worker, before the main Less modules are imported and can run.

To handle the problems surrounding the require call:
It is possible to replace the pluginLoader with another loader that is webpack compatible via a module alias that can redirect less/lib/less-browser/plugin-loader to whatever module of your make is holding your webpack-compatible version.

What you then need to do is replace the unbound variable reference to require that is here:

var PluginLoader = function(less) {
this.less = less;
// shim for browser require?
this.require = require;
};

— with a reference to an AMD compliant loader that is capable of working inside web workers.

@joepvl
Copy link

joepvl commented May 1, 2018

Thanks for the response, @rjgotten! I didn't know you could use a loader to inject things to be globally available to imported modules. Alternatively, I guess imports-loader combined with module.rules could be used to inject the globals, although it looks like webpack-contrib/imports-loader is kind of outdated.

That leaves us with the this.require = require issue. Aliasing the module should work, and I would go even further by saying that you could simply change the line to this.require = function() {} unless you actually need dynamic require support within any of your plugins, since it seems to me the only reason it's there is to be able to pass it to eval'd plugins here:

try {
loader = new Function("module", "require", "registerPlugin", "functions", "tree", "less", "fileInfo", contents);
loader(localModule, this.require, registerPlugin, registry, this.less.tree, this.less, fileInfo);
} catch (e) {
return new this.less.LessError(e, imports, filename);
}

If this is indeed the case, then I think it's worth considering removing it, because nobody may ever use it to begin with. IMO people could just be expected to take care of it within their plugins themselves — after all, the docs don't advertise require magically being available within plugin code, right? But maybe I'm missing something here with respect to the original intent of the code, and/or how this is supposed to work.

I'll probably test these things in the near future, if only for the satisfaction of it. 😉

@dasdeck how did you get on?

@rjgotten
Copy link
Contributor

rjgotten commented May 2, 2018

I didn't know you could use a loader to inject things to be globally available to imported modules.

You might not even need a dedicated loader to inject globals into the worker source here.
I think you can rely on the fact that imports are resolved in-order and change your worker's module to:

self.window = require( "window-shim" );
self.document = require( "document-shim" );

var less = require( "less" );

That will create window and document as members of the worker's top-level object, which should be enough to have them be used by the less compiler, as otherwise unbound variable references should translate to properties on the top-level object.

@matthew-dean
Copy link
Member

matthew-dean commented Jun 24, 2018

@joepvl @dasdeck FYI, this line

this.require = require;

...is removed in this PR - #3229

@matthew-dean
Copy link
Member

matthew-dean commented Jun 24, 2018

TL;DR - The in-scope require override exists in 3.0 for backwards-compatibility reasons. See: less/less-meta#17

@joepvl

the docs don't advertise require magically being available within plugin code, right? But maybe I'm missing something here with respect to the original intent of the code, and/or how this is supposed to work.

So, the over-ride of require in the scope of a plugin was a bit of a hack on my part to unify what had become essentially two separate plugin loading mechanisms.

  1. Plugin System A - Plugins were passed directly to the Less object (via API) and returned an object which had an install() method which was called.
  2. Plugin System B - Plugins with @plugin just, like, ran, man. There was no registration. And certain vars were included in the scope by evaluating it as a new Function(). It was not a well-thought-out feature when it was merged in 2.x.

So, I did months of work to essentially bridge the gap. The goal was to let @plugin load whichever kind of plugin, including Node.js ones. But Node.js plugins were loaded via require(), and sometimes had a bunch of files with require() calls. It also made them fairly non-portable to the browser.

Probably I could have made life easier on myself by just killing the previous @plugin implementation. Instead, imported plugins are all eval'd the same way, regardless of whether you pass them via API, or include them with @plugin. That means Node.js plugins are eval'd with a new function scope. And THAT means that require() would fail, because we haven't require()d the initial file. We've read it off the disk and eval'd it in a function. Sooo we have to essentially over-ride Node.js's require in scope for that plugin with a function that makes require calls work relative to a filename.

Incidentally, this is kind of what Node.js does anyway. That is, a require() call is always scoped to the file, so that it takes into account the local path.

So that's the long and short. In an effort to be backwards compatible, and not have two COMPLETELY DIFFERENT types of Less plugins, that's why that exists.

Moving forward, the best option is to:

A. Write or compile & distribute your plugins as a single file.
B. Use @plugin syntax only for importing Less plugins.

If you want to see the history / discussion on the two plugin systems and how they were unified, you can go here: less/less-meta#17

@joepvl
Copy link

joepvl commented Jun 26, 2018

@matthew-dean thanks for taking the time to explain, I appreciate it!

@stale
Copy link

stale bot commented Oct 24, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 24, 2018
@stale stale bot closed this as completed Nov 23, 2018
@eight04
Copy link

eight04 commented Nov 24, 2018

Here is a browserified bundle that can be used in the browser directly:
https://github.com/openstyles/less-bundle

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

No branches or pull requests

6 participants