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

Provide support for lesscss plugins #40

Merged
merged 1 commit into from Mar 22, 2015

Conversation

Projects
None yet
3 participants
@mfellner
Contributor

mfellner commented Mar 15, 2015

Since version 2 of less, Clean CSS is no longer bundled but has to be used as a plugin http://lesscss.org/usage/#command-line-usage-compress. The query-string configuration of less-loader is insufficient to pass plugin instances to less.

This change allows for plugins to be passed to less as specified here: http://lesscss.org/usage/#plugins-using-a-plugin-in-code.

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Mar 19, 2015

Member

You're right. It's better to use the webpack options in this case (although it's usually discouraged). What do you think @sokra?

Could you namespace and rename the option key to lessPlugins since it's actual a config for plugins. Like this:

module.exports = {
    lessLoader: {
        lessPlugins: [...]
    }
};

We never used the term lesscss before and afaict it's only their URL, not their actual module name.

Member

jhnns commented Mar 19, 2015

You're right. It's better to use the webpack options in this case (although it's usually discouraged). What do you think @sokra?

Could you namespace and rename the option key to lessPlugins since it's actual a config for plugins. Like this:

module.exports = {
    lessLoader: {
        lessPlugins: [...]
    }
};

We never used the term lesscss before and afaict it's only their URL, not their actual module name.

@sokra

This comment has been minimized.

Show comment
Hide comment
@sokra

sokra Mar 19, 2015

Member

You're right. It's better to use the webpack options in this case (although it's usually discouraged). What do you think @sokra?

yes. If you want to be perfect allow to choose the key in the options by query parameter. This way you could avoid conflicts.

Member

sokra commented Mar 19, 2015

You're right. It's better to use the webpack options in this case (although it's usually discouraged). What do you think @sokra?

yes. If you want to be perfect allow to choose the key in the options by query parameter. This way you could avoid conflicts.

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Mar 19, 2015

Member

If you want to be perfect allow to choose the key in the options by query parameter

That would be the perfect solution™ indeed 😀. But it should use a sane default like the npm name camelcased.

Something like:

require("!less-loader?config=someCustomName!./main.less")
module.exports = {
    someCustomName: {
        lessPlugins: [...]
    }
};

And config defaults to lessLoader.

Member

jhnns commented Mar 19, 2015

If you want to be perfect allow to choose the key in the options by query parameter

That would be the perfect solution™ indeed 😀. But it should use a sane default like the npm name camelcased.

Something like:

require("!less-loader?config=someCustomName!./main.less")
module.exports = {
    someCustomName: {
        lessPlugins: [...]
    }
};

And config defaults to lessLoader.

@mfellner

This comment has been minimized.

Show comment
Hide comment
@mfellner

mfellner Mar 22, 2015

Contributor

@jhnns, @sokra: I committed the suggested changes. Let me know if there are any other improvements I could make! 😄

Contributor

mfellner commented Mar 22, 2015

@jhnns, @sokra: I committed the suggested changes. Let me know if there are any other improvements I could make! 😄

jhnns added a commit that referenced this pull request Mar 22, 2015

Merge pull request #40 from mfellner/master
Provide support for LESS plugins

@jhnns jhnns merged commit 5d57637 into webpack-contrib:master Mar 22, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Mar 22, 2015

Member

Nice! Good work 👍

Shipped with 2.2.0

Member

jhnns commented Mar 22, 2015

Nice! Good work 👍

Shipped with 2.2.0

@mfellner

This comment has been minimized.

Show comment
Hide comment
@mfellner

mfellner Mar 22, 2015

Contributor

Thanks! Glad I could help.

Contributor

mfellner commented Mar 22, 2015

Thanks! Glad I could help.

@jhnns jhnns referenced this pull request Sep 11, 2015

Merged

functions option support #152

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