Closed broofa closed 2 years ago
@broofa this is amazing! So helpful for our usecase. Forgive my ignorance with JS, but we currently invoke this module via:
node_modules/less/bin/lessc
Would this PR introduce a command line arg like this disablePlugins=true
?
If not can it be added or can I help you add it?
Thanks!
Would this PR introduce a command line arg like this
disablePlugins=true
? If not can it be added or can I help you add it?
@matthew-gill heh... I actually misread your comment as "can you help add it" and so just added support for a --disable-plugins
flag in the lessc
command. For example:
$ cat foo.less
@plugin "autofix";
$ ./bin/lessc --disable-plugins foo.less
SyntaxError: @plugin statements are not allowed when disablePlugins is set to true in /Users/kieffer/codepen/less.js/packages/less/foo.less on line 1, column 9:
1 @plugin "autofix";
2
That said, we (CodePen) don't have an immediate need for this; we're just using the programmatic JS API. I'm happy to revert the CLI flag support. (Might be a good idea, to avoid complicating the CLI interface unnecessarily?)
Thoughts?
@broofa oh wow! Thanks for doing this! As this is against the upstream less.js, I'd LOVE to keep this in if that's ok? If your PR gets merged, this'll help us tremendously. Appreciate the efforts 👏
where are the docs in this repo?
@broofa https://github.com/less/less-docs , docs repo is standalone. Thanks for your contribution, great idea and implemention. 👍
I'd LOVE to keep this in if that's ok?
Of course. Is anything else needed here, then? Would love to see this merged and published sometime in the not-too-distant future, but it's not urgent.
Thoughts on how difficult this would be to backport to less@3
? That's the version our CodePen fork is off of. If we can get this on a 3.x release, we'd be able to just do a minor update to remove the fork dependency.
We'd love it backported to 3 too - we're in the same boat as you @broofa.
(P.s. thank you for working on this, it's really appreciated!!)
@matthew-dean: Are you the right person to nag about the status of this?
I've discovered yet another down-side to being on our own fork of less
: It prevents us from leveraging the community defined TS declarations in @types/less
(... because we're importing this as "@codepen/less" instead of "less", which breaks the inferred @type/
Hi all. A few notes about this.
One is that probably this should be disableInlinePlugins
or disablePluginRule
(whatever makes sense to people, maybe the latter?). The reason is that there are compiler-level / configuration plugins and it's ambiguous as to which type of plugin inclusion you're referring to. (Unless you truly want to disable all plugins? But then, does this do that?)
Next, yes, I would support it at the lessc level. Ideally, all option flags in lessc
should have parity with the JS object options.
Finally, re: backporting to 3.0. The main reason for the 4.0 release is that math=parens-division
is the default option, which is technically a breaking change. (See: https://lesscss.org/usage/#less-options-math)
It's basically the same as 3.0 other than that. So, no, I don't think backporting is a good option. What I would do for CodePen, if it were me, is to not make this breaking change, just use 4.0, but set Less's options to be math=all
for existing pens, and then use the default setting (parens-division) for new pens, as it results in less developer error, especially as /
has been added over the years to more and more CSS prop values.
@matthew-dean
One is that probably this should be
disableInlinePlugins
ordisablePluginRule
(whatever makes sense to people, maybe the latter?). The reason is that there are compiler-level / configuration plugins and it's ambiguous as to which type of plugin inclusion you're referring to. (Unless you truly want to disable all plugins? But then, does this do that?)
I've renamed disablePlugins
-> disablePluginRule
, and the CLI flag to --disable-plugin-rule
.
Next, yes, I would support it at the lessc level. Ideally, all option flags in
lessc
should have parity with the JS object options.
👍
Finally, re: backporting to 3.0. The main reason for the 4.0 release is that
math=parens-division
is the default option, which is technically a breaking change. (See: https://lesscss.org/usage/#less-options-math)It's basically the same as 3.0 other than that. So, no, I don't think backporting is a good option.
Sounds good. I'll run this by folks here to see if there are any issues on our end and circle back if so. In the meantime, if we could get this out for 4.x, that would be awesome.
Let me know if you need anything else here.
Sounds good. I'll run this by folks here to see if there are any issues on our end and circle back if so. In the meantime, if we could get this out for 4.x, that would be awesome.
FYI, the consensus here (codepen) is that we can work around the math
change on our end. (tl;dr: we'll just set math = 'always'
for legacy pens to retain the 3.X behavior.)
@matthew-dean Sweet! Thanks for taking this.
Any idea when the next release will go out?
Is there any reason to disable both --plugin
and --disable-plugin-rule
at the same time since it is not disablePlugins
anymore?
I would like use plugins enabled from outside (CLI or options), but keep @plugin
disabled.
What:
Adds support for a
options.disablePlugins
flag on theless.render()
method. Setting this to true disables parsing of@plugin
statements and, instead, throws an error.Why:
As
@plugin
statements may be used to run arbitrary system commands. This is a significant security risk for applications that need to process untrusted code. At a minimum, applications should be able to disable parsing of@plugin
statements.As noted by my colleague @shshaw here at CodePen, we ended up forking this repo so we could disable
@plugin
processing. Adding this option will allow us and our users to stay current with the officialless
codepase.See also:
How:
Calling
less.render(inputString, {disablePlugins: true})
activates the new codepath. With this option enabled, theparsers.plugin()
method is replaced with one that failes with an error when an@plugin
statement is detected.Checklist: