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

3.0 Branch Updates #20

Closed
21 of 31 tasks
matthew-dean opened this issue Jul 2, 2016 · 20 comments
Closed
21 of 31 tasks

3.0 Branch Updates #20

matthew-dean opened this issue Jul 2, 2016 · 20 comments

Comments

@matthew-dean
Copy link
Member

matthew-dean commented Jul 2, 2016

This is a tracking list of changes for the 3.x branch.

3.0 Change List

General

  • Directive renamed to AtRule, Rule renamed to Declaration (See: On the Less API #15) less/less.js@52e9b5e
  • Inline JavaScript disabled by default (vulnerable to XSS attacks)
  • Deprecated less.postProcessor option removed. (PostProcessor plugins are still fine.)
  • Property accessors (See: Feature request: Property lookup (a.k.a Properties are Variables too) less.js#2433)
  • Nodes now have a parent property, which allows:
  • Plugins / functions which create nodes without indexes / fileInfo can inherit that up the chain (simplified user API)
  • less-node's file manager now adds NPM pathing (resolving node modules) by default. NPM resolving was first moved from lessc to the plugin manager, but it made more sense to make it generalized to file imports if it was going to be there at all
  • simple boolean if() function added with PR #3079

Plugin changes

  • Unified plugin format (See: Unification of plugin subsystems. #17) - Now NPM-installed plugins can be called with @plugin! Also aligned is the auto-searching behavior of lessc. That is, invoking @plugin "clean-css"; will attempt (in a Node environment) to load "less-plugin-clean-css" and "clean-css" modules, before looking in the local directory (same file path as invoking .less file) for clean-css.js and less-plugin-clean-css.js. The browser environment will of course only look for the last two single file matches.
  • Added support for arguments for @plugin - @plugin (args) "plugin-name". Like lessc, args is an arbitrary string
  • Validation of plugins that was previously only available to lessc is now available to Node API & browser environments (such as minimum Less version and invoking the plugin as a function if detected) - Fixes "plugin.install is not a function" when trying to add plugins to node script less.js#2915.
  • Functions in the function registry can now return false, such that a function does not return a Node. This allows plugins to register functions which serve purposes other than Node creation.
  • The install() function is now optional. It will be invoked once for the first @plugin call.
  • Plugins can optionally expose a use() function. This gets called when the @plugin statement is first evaluated (pre-eval of the Less tree).
  • Plugins invoked inline can optionally expose an eval() function. This gets called during Less's evaluation cycle. The purpose for this is that plugins can create scoped nodes and immediately evaluate them in the current context.
  • Inline plugins can either use registerPlugin({...}) or CommonJS-style module.exports = {...} with the plugin object signature. However, plugins are now not required to return anything. Planning on changing this based on feedback.
  • Plugins can set a minimum Less version in string format, rather than just an array ("2.3"). However, I'll probably leave this out of documentation for backwards compatibility, since past versions of Less will expect an array. This is just future-thinking.
  • Visitors added by plugins while Less is evaluating @plugin will still be visited. The plugin manager now returns a dynamic visitor iterator.

3.0 Tests needed

  • Tests for seeing that Browserify'd Node plugins work seamlessly with @plugin Not a compatible system
  • Tests to make sure that browser plugin calls are not vulnerable to XSS attacks (limited to same-domain?)
  • Tests for sending arguments to @plugin Moved to 3.1
  • Tests for making sure errors thrown in plugins bubble into appropriate Less errors
  • Add line/column reporting for anonymous plugin function
  • Add try/catch to use/eval calls
  • Add try/catch to install call
  • Add try/catch to custom function calls

To Deprecate in 3.0

  • Pre-processor function (not the same as pre-processing plugins)
  • HTML rendering of errors in browser mode. Sophisticated JS consoles are now in common use and did not exist when this was added.
  • Intent to deprecate - pre-processing plugins. It's the only thing that prevents simplification of @plugin syntax. As far as I can tell, this is mainly used to import libraries by injecting text with an @import at-rule. There has to be a better way to simply add imports at the point of @plugin evaluation, but I haven't been able to figure it out. If it's to perform other conversions of styles before Less evaluation, there are plenty of other better-supported tools that do this, such as Gulp or PostCSS.
  • Intent to deprecate - post-processing plugins. (This is different from the above less.postProcessor option, which was a specialized option.) This feature was created to add Autoprefixer and Clean-CSS support, but now there are much better task runners (Grunt / Gulp, etc) - See: Error: ENOENT with lessc & plugin-autoprefix & plugin-clean-css less.js#2612 (comment)

Move to 3.1

  • Details, friendlier options, and tests for sending arguments to @plugin (like @plugin(arg: val)?)

Move to 4.0

  • New variable / property name-spacing syntax (See: Variable / property namespacing #12)
  • Remove JavaScript back-tick support (vulnerable to XSS attacks)
  • Remove features deprecated in 3.0

I'll keep this updated as proposals get finalized.

@matthew-dean
Copy link
Member Author

The 3.x pre-release branch is now published to NPM under the tag "next", per convention.

@rjgotten
Copy link

rjgotten commented Jul 20, 2016

Plugins invoked inline can optionally expose an eval() function. This gets called during Less's evaluation cycle. The purpose for this is that plugins can create scoped nodes and immediately evaluate them in the current context. (Not sure if this is needed since I realized that functions that are called are bound to an object with this.context attached. I had this come up and I haven't tested the difference. If that's sufficient, I could probably remove this.)

Having a hook into the formal evaluation cycle means you offer users a means to avoid having to write some quite tricky code to create the evaluation tree by hand. That's always a good thing, imho.

@matthew-dean
Copy link
Member Author

@rjgotten You're right. It's a fairly simple thing to include, and the context for @plugin may, of course, be different than the direct context for a function call, because of scoping.

@matthew-dean
Copy link
Member Author

Did much of the work over Christmas break. Just need to add tests for sending arguments to plugins. 3.0 alpha 1 is released under the "next" tag on NPM.

@matthew-dean
Copy link
Member Author

Wrote a ton of documentation tonight. If anyone wants to help me out, that would be great.

Example: https://github.com/less/less-docs/blob/3.x/content/features/plugins.md

@rjgotten
Copy link

rjgotten commented Jun 6, 2017

There has to be a better way to simply add imports at the point of @plugin evaluation

Better? Yes.
Easier? Not quite.
Just as performant? Hell no!

Architecturally, the best way to do it would be to chain the evaluation of the AST together using a Promise implementation. When all node evaluations complete asynchronously, any node evaluation may undertake asynchronous actions such as downloading and processing additional @import files.

It would also enable functions that require data asynchronously, which could be interesting.
And if you really want to get experimental: you could distribute work over multiple DOM worker threads in the browser or the functional equivalent of your choosing for the Node.js environment.

Processing @import files in parallel might actually tip the compiler into being more performant again, since it could benefit from multi-core processors to distribute work and can process an already loaded @import file's source while others are still busy downloading or being streamed into memory from disk.

It would be a radical, ra-di-cal departure from the current compiler though. So not something I would attempt to undertake until a next major version.

@seven-phases-max
Copy link
Member

seven-phases-max commented Jun 6, 2017

Processing @import files in parallel might actually tip the compiler into being more performant again,

Cool and shining until the very moment one import uses data from another, then all that cryptic async stuff becomes totally useless.

@matthew-dean
Copy link
Member Author

@rjgotten - I don't get it. If an @import can have @imports that can resolve and evaluate correctly, and an @import can have vars that need to be evaluated before the import is resolved, why can't a @plugin produce @imports? Why does it require a massive re-write? It sounds like you're over-thinking it. There's gotta be a way for a plugin to register imports using already-existing code. I just don't understand the import code enough yet to figure it out. My guess is all the pieces are there already.

@rjgotten
Copy link

rjgotten commented Jun 7, 2017

@matthew-dean
A plugin can add imports as part of the plugin's own registration when the @plugin declaration is processed and completed async. But if you try to do conditional stuff or try to dynamically add imports e.g. when a function registered by the plugin is called, you could land in hot water.

@seven-phases-max

Cool and shining until the very moment one import uses data from another, then all that cryptic async stuff becomes totally useless.

Hence: 'might'. Ofcourse not everything parallelizes as well as the ideal.
(And yeah; I do agree that most async stuff in Node is mindblowingly weird.)

@matthew-dean
Copy link
Member Author

matthew-dean commented Jun 7, 2017

A plugin can add imports as part of the plugin's own registration when the @plugin declaration is processed and completed async.

Okay, how? I tried to get it to work and I never could. I could create an import node, but I could never get it to do anything (as in, actually import a Less file). Can you test this? And once you verify that it's possible, can you add tests into the repo?

@rjgotten
Copy link

rjgotten commented Jun 7, 2017

I meant from a control flow perspective it's possible.
I didn't mean the logic to actually do it is already there.

Sorry for the confusion.

@matthew-dean
Copy link
Member Author

I pushed v3.0.0-alpha.3 today.

Question on my mind: how forward-looking is the plugin syntax considering import/export (ES6 module syntax)?

@rjgotten
Copy link

rjgotten commented Oct 9, 2017

I don't think the ES6 module syntax can be easily unified with AMD/CJS syntax in a single runtime file.
Also; if you also want to support ES6 modules natively in Node.js you're stuck with a separate .jsm file anyway, last I checked.

Maybe the correct way forwards here is to do away with both AMD and ES6 module syntax and go a different path: enable Less to run from within a web worker. That way you can use a combination of CJS require() and the web worker importScripts(), both of which are synchronous.

This would also lessen the load on the main UI thread of the browser, which could be desirable to speed up start-up and reduce stutter when using hot reloading.

(And a considerable bonus: importScripts is, iirc, subject to cross-domain policies, meaning you're protected against several flavors of cross-site scripting attacks.)

@matthew-dean
Copy link
Member Author

matthew-dean commented Oct 10, 2017

I think any XHR script requests should be protected from XSS already, no? I couldn't ever get clear feedback about this. Whether we needed special XSS protection in @plugin or not. I don't have the time to research / test it.

As for this:

Maybe the correct way forwards here is to do away with both AMD and ES6 module syntax and go a different path: enable Less to run from within a web worker.

O_O

I mean, sure, but... ⌛️

Also, if Less is going to be re-written for speed, it should probably be done in Web Assembly, lol.

Either way, I'm not sure how web workers address plugins or the correct way people should be including / calling them.

You know.... technically, in the browser, all we should really allow in v3.0+ is @plugin. That would rid us of all that UMD crap since we're retrieving the file and defining exactly how it's being evaluated. But we better be sure that XHR is not XSS-vulnerable. It's not, is it? This is something I should know more about but don't.

@rjgotten
Copy link

rjgotten commented Oct 10, 2017

XHR is not vulnerable. However, AMD modules typically are not loaded via an XHR transport. They're loaded as script tags.

As for

Either way, I'm not sure how web workers address plugins or the correct way people should be including / calling them.

Web workers have a built-in synchronous way of loading scripts: the importScripts() function.
CommonJS has a built-in synchronous way of loading scripts: the require function.

The two paired up works nicely. You can do away with asynchronous dependency resolution and all its complications, and the browser doesn't need a full module loader baked into Less, since the built-in function already does most of the heavy lifting. You just need a little bit of paving over its top to shim a functional subset of require to fetch sub-dependencies.

Simple single-file plugins wouldn't even need to fetch dependencies and for those all of this is really a theoretical discussion to begin with, I think.

@matthew-dean
Copy link
Member Author

Web workers have a built-in synchronous way of loading scripts: the importScripts() function.
CommonJS has a built-in synchronous way of loading scripts: the require function.

The two paired up works nicely. You can do away with asynchronous dependency resolution and all its complications, and the browser doesn't need a full module loader baked into Less, since the built-in function already does most of the heavy lifting.

Hmm... I didn't know this. Node.js doesn't have web workers (although of course there are libs to simulate), and if you used require, it'd be in the same thread, different from web workers. But I guess single-thread in Node.js is not a big deal since it's not performing a bunch of extra work like the browser is.

How do you feel about this:

technically, in the browser, all we should really allow in v3.0+ is @plugin

@rjgotten
Copy link

In the browser only allowing @plugin to register plugins seems ideal from a control point of view: Less determines how the plugin file is loaded.

But you'd need to make it very clear to plugin authors that they should not be assuming CJS require(); AMD define(); ES6 import etc. to be available for loading further sub-dependencies. That is: all plugins loaded in browser should be single-file only.

@matthew-dean
Copy link
Member Author

@rjgotten I removed UMD definition from Less.js and less-docs for v3.0.0-RC.1. I updated the docs to say that @plugin is basically the only supported option in-browser.

@matthew-dean
Copy link
Member Author

If there are no complaints, I'll merge RC1 into master sometime this week and publish 3.0.

@matthew-dean
Copy link
Member Author

Closing as 3.0 is released. Any future 4.0 thread can pull/reference from this one.

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

No branches or pull requests

3 participants