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

Unification of plugin subsystems. #17

Closed
seven-phases-max opened this issue Apr 30, 2016 · 60 comments
Closed

Unification of plugin subsystems. #17

seven-phases-max opened this issue Apr 30, 2016 · 60 comments

Comments

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 30, 2016

As was already mentioned initially (and then in #10 and in various topics here and there), @plugin feature needs further improvements as its current implementation was just a temporary solution to get something in the release ASAP.

So the very first step for the unification would be to allow the @plugin directive to load both formats, i.e.:

@plugin "clean-css";
  • first try to load the "standard" format, i.e. less-plugin-clean-css (in the directories specified in the docs)
  • and only then try to load clean-css.js file (of the "easy-functions" format, in the directories of the compiled less project).

Contrary:

@plugin "clean-css.js";

looks directly for an "easy-functions" clean-css.js file.


As also mentioned earlier, loading "standard format" plugins with @plugin directive is expected to have certain acceptable limitations/restrictions to work properly (e.g. no special scoping and the directive should appear before any code it may have an effect on (specifically for "visitor" plugins)).


Setting "standard format" options via @plugin are yet to be specified later. Obviously the options will be set via typical @import/@media-like syntax, e.g.:

@plugin (s1, advanced, compatibility: ie8) "clean-css";

But exact implementation details to be set up later.

@matthew-dean
Copy link
Member

Good start. I'm actually less familiar with what you call the "standard format" plugins because the usage always seemed overly complex, so I never bothered with it. But @plugin plugins are a no-brainer. They're easy to write and they always work. Part of the reason they're easy is because they're limited, which is actually a good feature.

In any case, I don't see anything in the docs pertaining to directory location of the old plugin format. The plugin file actually has to be "pre-loaded", does it not? So I don't think Less looks for it anywhere. Whereas @plugin just follows typical @import Less rules. The plugin file should be with your "Less project" somewhere, which you need to reference explicitly.


Having @plugin do a special search first to prepend "less-plugin" on the name of the file seems like a regression. IMO it should continue to follow current @import/@plugin behavior of searching exactly the file you just wrote, in the relative or absolute directory you specified (barring any modifications of directory searching in your initial Less options).

If an author wants to follow a convention of using "less-plugin-" but wants to shorten the reference, they can just easily write:

@p: "less-plugin";
@plugin "@{p}-clean-css";

Or whatever else they want to do there. Adding two specific words in the name of a JavaScript file doesn't make sense to make a dedicated feature, since names of files are arbitrary. AND, if they want plugins to do such prepending automatically, they can just add a "less-plugin-" prepending plugin! 😉

@plugin "less-plugins";
@plugin "clean-css";

(Although the latter wouldn't be recommended, since it would be altering any plugin reference used anywhere. But still, it's an option.)


I agree that adding "options" to plugins can be discussed later.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 1, 2016

Having @plugin do a special search first to prepend "less-plugin" on the name of the file seems like a regression

It depends. Here you're thinking (as @rjgotten also most likely did when submitting the initial @plugin PR) as someone who is just about to write and use his own project specific custom functions. But (not putting any IMHO, but it's obviously assumed anywhere anyway) the general auditory is just about:

  • npm install (-g) less-plugin-stdlib (the prefix is actually optional and used only to easily identify Less plugins among other nmp-modules)
  • @plugin "stdlib";

We can get back to less/less.js#1861 (comment) and start all over again from scratch of course. But this is how the "standard format" works (lessc --stdlib) and I don't see any reason for @plugin to not follow this convention.

searching exactly the file you just wrote

@plugin "path-to/the-file-you-just-wrote.js";

In any case, I don't see anything in the docs pertaining to directory location of the old plugin format.

Then it is the issue of the docs to be fixed.


Also speaking of the "old format". I guess we have to clarify this. The "standard format" is no way "the old" but actually the one and the only format. You may be less familiar with it but it's the only way by now to make something beside custom functions, so I recommend you to get more familiar with it and start looking where it has to be improved to meet your vision. Otherwise you're risking to continuously reinvent the wheel by proposing parallel incompatible solutions (like less/less.js#2852).

In other words, it's not about making the @plugin to be able to do what "standard format" can do, but just about making the current @plugin implementation to conform to the "standard format". Otherwise I'm afraid we definitely need to stop, disable both features, get back to less/less.js#1861 and start redesigning everything from scratch.

@matthew-dean
Copy link
Member

Also speaking of the "old format". I guess we have to clarify this. The "standard format" is no way "the old" but actually the one and the only format. You may be less familiar with it but it's the only way by now to make something beside custom functions, so I recommend you to get more familiar with it and start looking where it has to be improved to meet your vision.

That's fair. But what I meant was that we had talked about moving plugins to be loadable under @plugin, and I wasn't sure that the "standard format" was compatible with that model.

But this is how the "standard format" works (lessc --stdlib) and I don't see any reason for @plugin to not follow this convention.

This is also a good point. I see where you're coming from. I had assumed that a) backward compatibility was not possible because of the way plugins are loaded, b) the solution might have to be to deprecate plugin format A, and have plugin format B (@plugin) have the same technical capability as the current format but I wasn't sure that it could be compatible with the way plugins are currently written.

But, you're not seeing them as "two formats", so much as a newer way (and maybe easier way) to call a Less plugin.

In other words, it's not about making the @plugin to be able to do what "standard format" can do, but just about making the current @plugin implementation to conform to the "standard format".

I hear you. Thank you for clarifying. One advantage we have is that I think we can classify the current @plugin implementation as "experimental" (since it's not yet documented), and so therefore it shouldn't need any official "deprecation" for any possible breaking changes (although it would be nice to avoid).

@matthew-dean
Copy link
Member

One comment about loading order, though, as far as to which one is first. I still think the "explicit name" should be checked first. The original docs say, "We recommend the plugin starts "less-plugin" though that isn't required."

"less-plugin" is supported, but it still seems like a convenience addition that it's checked after the explicit name is checked, as the name is an optional recommendation. So I think the name check would happen more like:

@plugin "clean-css";
  1. Check for npm installed "clean-css" plugin (or loaded file in the browser?).
  2. Check for "clean-css.js" relative to the Less file that just called it.
  3. Check for npm installed "less-plugin-clean-css" plugin.
  4. Check for "less-plugin-clean-css.js" relative to the Less file that just called it.

There's bridging @plugin to current plugin implementations, but there's also bridging standard Less plugins to other file referencing conventions in Less. IMO convenience methods should typically be a fallback.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 1, 2016

Well, my proposal of the order is based solely on "the better performance for the masses" sense of view. Yet again assuming the majority of users will use "public/shared" modules (which will have the prefix for obvious reasons) instead of self-written plugins, it would make sense to not burden the majority with additional search step that rarely matches anyway.
(Though with or without prefix and with or without extension, it still would make sense to first look for the local installation and only then for the global. Now the only problem is to define what is "local" in relativity to the current Less file (unlike node, Less does not have a concept of the "project directory" (usually it's assumed to be the directory of the master less file but it's not always the case)).

The only concern is the "browser-side" plugins: obviously since a node-based plugin has to be browserified somehow before to be used client-side, and that browserified version also ends in plugin.js file we have to introduce some artificial marker to distinguish between two formats (assuming we'll keep the "easy functions" format at least for the backward-compatibility even if it's totally overlapped with the "standard format").

@matthew-dean
Copy link
Member

matthew-dean commented May 1, 2016

Yet again assuming the majority of users will use "public/shared" modules (which will have the prefix for obvious reasons) instead of self-written plugins, it would make sense to not burden the majority with additional search step that rarely matches anyway.

A public/shared module does not automatically mean an "npm-installed" one. The jQuery plugin ecosystem does not rely on Node as a dependency, and I don't think Less should either. Officially, Less is platform-agnostic. To me, that means that "core Less" should assume nothing other than the file reference you gave is a valid, local file. True, the platform extensions can then define what is "valid and local". So, it would make sense that the Node platform extension might have a different search mechanism / order than what happens in-browser.

As an example that you're drawing from for convention: lessc is a platform-specific Node extension of Less.js, albeit an official one. So, by definition, the features of lessc are not universal, nor are the conventions.

I think, conventionally, Less plugins have basically been "Node-only", partly because they rely on NPM, and partly because they're written in such a way that excludes the browser (relies on Node as a dependency). I think that's part of what has limited plugin adoption and ease-of-use, so I wouldn't want to see that entirely as a guiding principle.

For instance, there are currently "Less plugins" that rely on NPM, explictly say they don't work in the browser, but are nothing more than additional Less functions. That's wholly unnecessary. I think for a successful plugin ecosystem, we should guide developers to create platform-agnostic Less plugins, since Less itself is platform-agnostic. Just like there were mistakes made with the initial @plugin implementation, I believe there were mistakes made with the initial plugin implementation in 2.0.

Which certainly doesn't mean that we should break those plugins on a whim. I just want to really focus on:

  1. What makes plugins easy to write
  2. What makes plugins easy to use

I think those are the two most-essential pieces of a successful plugin system. If, in our documentation, we give examples that suggest, "It's okay to create a Node-only plugin", then we're effectively saying that Node is really the only valid Less environment, which makes non-Node/NPM plugin scenarios harder to use.

So as to:

The only concern is the "browser-side" plugins: obviously since a node-based plugin has to be browserified somehow before to be used client-side

I think we should caution people against making "Node-side" or "browser-side" plugins as much as possible. Or at least thinking if such a thing is absolutely necessary. Rather than thinking of Less as a Node-based system that you add to with NPM-hosted plugins, it might be easier for some users to treat Less plugins like jQuery ones, where the plugins travel with the associated Less project, not that get installed somewhere "in-system", as those have dependencies which are not portable. Less plugins can easily exist as a single JS file which gets copied into your local Less source file directory.

But, the nice thing is that if we do things right, we don't have to adopt either "philosophy". Someone could choose to manage their Less plugins via NPM, and another person could copy and paste a file. If we write guidelines and examples appropriately, people can have the best of both worlds.

@matthew-dean
Copy link
Member

Just as an example as far as cross-platform plugin conventions, I've seen a library like Ractive (which can run in both browser and Node), basically do conventions as:

  • Plugins either exist as a single file, or include a dist/ directory with a single file that can be used in-browser (similar to what Less itself does)
  • Plugins basically have Node consumption and browser consumption instructions as a standard part of each plugin repo

One thing that PostCSS does is it has a plugin generation script which creates your initial repo. So, we could do something similar where someone enters the name of their plugin, and it does the following:

  1. Sets up a package.json file, a dist/ folder, an initial index.js file (for NPM/Node), and both build and test scripts for the plugin
  2. Populates the documentation to demonstrate Node usage via "npm install" and via @plugin.
  3. Demonstrates in-browser use of the distribution file (possibly without the "less-plugin-" prefix? Or as an option when generating the repo?) with @plugin

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 1, 2016

a plugin generation script which creates your initial repo.

This is probably better to be done with something like Yeoman (though personally I'm not very familiar with it since it was broken for my dev-platform for a while).

@matthew-dean
Copy link
Member

Yeoman may be what was/is used, in fact. So, yep, something along those lines.

@rjgotten
Copy link

rjgotten commented May 2, 2016

Here you're thinking (as @rjgotten also most likely did when submitting the initial @plugin PR) as someone who is just about to write and use his own project specific custom functions.

Actually; I wrote @plugin the way I wrote it, to transparantly support plugins being included via transitive closure, i.e. , as dependencies of dependencies, without bothering the consumer (developer) of the Less file that imports the first dependency.

It was meant as a zero-setup, collision-free means to include custom functions for Less frameworks.

The zero-setup aspect makes it work transparantly with any side-by-side editor that calls out to Lessc to create CSS previews, without having to mess with custom settings. That's an additional perk and may be a good reason to also develop your own project-specific functions in the @plugin format.

we have to introduce some artificial marker to distinguish between two formats

Imports may use URLs, so why not @plugin "node://clean-css" to instruct the compiler to use the NodeJS module lookup rules.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 2, 2016

so why not @plugin "node://clean-css"

same mistake as:

A public/shared module does not automatically mean an "npm-installed" one.

Notice there's no mention of node or npm in the initial post.

@plugin "clean-css";

is fine to load PHP-written (optionally even particular platform-specific, e.g. Wordpress) less-plugin-clean-css in less.php for example.


It was meant as a zero-setup, collision-free means to include custom functions for Less frameworks.

This is illusion (no offense meant :) - it stops to be true as soon as you start to share the same plugin in different projects (so you either need a copy of the plugin in each or setup the shared paths, thus -> no zero-setup) or start to use different libraries that include different versions of the same plugin (so sooner or later someone comes with an idea of 'Less plugin manager/packager' utility).

So in general it's all the same problems whatever approach you choose.

@matthew-dean
Copy link
Member

matthew-dean commented May 2, 2016

This is illusion (no offense meant :) - it stops to be true as soon as you start to share the same plugin in different projects (so you either need a copy of the plugin in each or setup the shared paths, thus -> no zero-setup) or start to use different libraries that include different versions of the same plugin (so sooner or later someone comes with an idea of 'Less plugin manager/packager' utility).

Different versions of the same plugin are only an issue if you use NPM to manage your plugins. NPM is famously terrible and being able to install multiple versions to use simultaneously.

The way @rjgotten wrote it seems like it would work fine in that scenario, since functions are scoped, and the included JS file doesn't contain any sort of unique identifier that can collide if you have multiple copies. Therefore, you should be able to do:

#library1 {
  @plugin "library1/foo-plugin.js";
  bar: foo(); 
}
#library2 {
  @plugin "library2/foo-plugin.js";  // same plugin, different version
  bar: foo();
}

Standard/current format plugins modify Less at the core/root level, and so are inherently prone to collisions in the way you describe. Currently @plugin functions are not. It's one of the reasons that I feel that the copy/paste of a distribution file is actually the more appropriate recommended method. It can get included in the source files right where you need it.

And then you have everything that "builds" your final CSS file all in one place. It's not pulling your Less file from a local directory and then a JS file from some other place, even though they have similar URL paths.

The way I visualize this is like:

screen shot 2016-05-02 at 10 19 15 am

@matthew-dean
Copy link
Member

matthew-dean commented May 2, 2016

I also agree though that this is probably the wrong way to go:

@plugin "node://clean-css"

Unless...... like the way that "less-plugin-" is magically prepended for lessc, we assign a meaning to the browser. (The browser knows to look in node_modules.)

That said, it's such a specific case that, again, the "node://" URL scheme should itself be supported by a plugin. With proper architecture and good documentation, we can keep the core light and support those kinds of customizations better.

I've said it before in other threads: there's no reason there can't be official plugins that pull in optional JS files. We have official platform extensions, so something that searches node_modules/ could be a supported plugin. A benefit to having official plugins is to help establish plugin convention.

@rjgotten
Copy link

rjgotten commented May 2, 2016

That said, it's such a specific case that, again, the "node://" URL scheme should itself be supported by a plugin.

Ah. You beat me to it. Yeah; that was going to be my argument as well.

Also, just for the record: Node.js module lookup is different from what Lessc is currently using. Lessc is using require in its plugin loader, alright, but it searches an opinionated number of folders in a non-standard order, rather than following the node module lookup algorithm. (Look up the maddening details of the lookup algorithm at your own risk.

So it should be called something different from node:// anyway.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 2, 2016

Well, it's not hard to guess what is the key differences between our visions. Some treat plugins as a method to extend a particular project, others think of it as an extension to the language/compiler to meet that project needs (hence first are biased towards local copy-pasted js files, second favor shared libs). And there's no way to prove anything but by practice itself.

And then you have everything that "builds" your final CSS file all in one place

Yep (for me there's no way to think of lists, colors or functions plugins as a part of a local project).

@matthew-dean
Copy link
Member

Yep (for me there's no way to think of lists, colors or functions plugins as a part of a local project)

For me, there's no way to think of those things but part of a local project, since the local .less code would have them as a dependency. Think of something like Bootstrap (obvs not Bootstrap 4). They already distribute the .less source files which you can include in your project. Now, what if a library like that added some custom functions in JS. If your library's .js is local to that source folder, zero steps and zero additional dependencies are added. If you instead require Node, then that's an additional step and an additional dependency. Plus (with standard format custom functions) your .less code will potentially produce incorrect output without generating an error. With @plugin, a failed build because of a missed step is impossible. If your .less code requires a library's plugin, and you've downloaded that library's source, and have imported it, it will always succeed. That was the brilliance of the @plugin concept in the first place.

Ironically, the existing format is like a PHP environment, where available libraries are not guaranteed unless they are installed at the system-level, which leads to the reality where PHP !== PHP. Node.js solved this by allowing packages to be installed to a local folder by default. Such that running that project is, by and large, reliable and consistent.

But, with current Less plugins, if "Awesome Less Library" relies on plugins, and you take your /less/awesomeless/ folder, and move it into another project, it will break. For this reason, Less libraries don't typically bother with Less plugins. They're not entirely reliable, and not exactly user-friendly. If it requires a specific Less.js setup in order for your library to be imported, you automatically raise the technical bar of who can use it.

@plugin with a local file doesn't suffer from any of those things. Even if there might be a learning-curve for adjusting to a different method of importing a plugin, why would you want to place that additional technical burden on Less users? Or the additional dependencies on Less library authors?

While you may have a preference for managing modules using a specific package manager, @plugin "local-file"; has no specific technical burden, other than perhaps updating a library. BUT, this leads to an even more important point, how do you manage those library dependencies when the .less code directly depends on the JS plugin version?

Let's say you have this:

@import "lib/awesomeless";
awesome();

// lib/awesomeless.less
@plugin "awesome-functions";
// ... stuff that relies on awesome-functions

Say Awesome Less Library is importing that plugin via NPM, and at some point releases a new version: The user updates the NPM plugin, but fails to copy the new .less files. They're using two separate methods to update their project dependencies, and they have to keep them perfectly in sync.

The question of how you manage that is rhetorical, because I'm sure smart brains here could come up with a clever method.

The point is: with @plugin and local files, it's simply not an issue. Your LESS source dependencies are all in one place. There is no clever method needed. Your bundle of .less and .js would never not work.

Now, ALL THAT SAID, the best scenario still may be to support either method. I'm obviously of the mind of which one should be recommended. I would prefer that the current plugin format migrated towards the @plugin design (as @rjgotten would probably be), and I think @seven-phases-max you would prefer that the @plugin design migrated towards the current plugin format.

Those two philosophies are not necessarily mutually exclusive. My hope is that there is a middle ground for all.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 2, 2016

Node.js solved this by allowing packages to be installed to a local folder by default.

This fixes one problem (aka "dll hell"), but introduces others. For instance it implicitly enforces "never update" approach simply because even with zillion tools to track/automate this, still even a minor update of a project with a lot dependencies becomes a disaster (see less-docs->assemble for a good example). So as I said above it's all the same problems with either method. And in this particular case an approach with re-using existing dependency managing stuff (like npm with all of its problems) looks like a short-term winner.
E.g. same Awesome example: assume you install it via bower or link through some CDN, then you find a bug in a plugin AS uses that does not affect AS but do affect your code. Now, if that plugin cannot not be "scoped" in anyway (e.g. autoprefixer/clean-css for example) you're doomed because at best you have to manually update the package that is not supposed to be modified (i.e. bower version) and at worst (CDN) you have to rework you dep. method.
So no, all this "local stuff" looks nice and shining only for a narrow set of use-cases... (Specifically if we consider that the "scope" itself still needs some parsing/evaluation before it comes into effect, thus ... Actually I already have an example of the corresponding issue even just for functions - and there're more to come as soon as there more varieties of @plugin/stuff-it-does things pop up).

Now, ALL THAT SAID, the best scenario still may be to support either method.

Yep, and that's just what #17 (comment) says. Technically our dispute is just about what @plugin "foo"; should search first... Should it be whatever "foo" plugin of that platform (e.g. npm one when in node) or right "foo.js" (with whatever or none platform dependencies it has). And then we again return to #17 (comment).

@matthew-dean
Copy link
Member

matthew-dean commented May 3, 2016

Some of that I didn't quite follow in the second paragraph because of some typos maybe? I couldn't quite decipher "cannot not scoped" and "plugin AS uses". But I think the gist is that you're talking about the benefits of a package manager.

There are probably pros and cons of each approach. So, what it comes down to for me isn't "should the user be using a package manager" but "should we force the user to use a package manager". To me, the latter is an obvious no. It would be better if we didn't force platform nor packaging, as Less.js itself does not. Is it okay if a plugin is only Node-specific? Sure. But if Node is not an absolute requirement, there's no reason for a plugin to not be agnostic, and I think demonstrating that to users would help bolster the platform.

So, let's step away from the philosophy of it because it's not getting anywhere. Let's accept that some people will want to use X plugin via npm install (or bower) and some will want to use X plugin via dist/my-plugin and design accordingly. Whichever one is most frictionless for a particular user depends entirely on their points of friction.

My suspicion is that some good examples of using @plugin will bring interest to Less plugins where there wasn't before, but how much, I'm not sure.

So, basically, my thinking is that:

  1. A @plugin call should be able to load an npm installed plugin if (decision needed) a) the Node plugin is loaded or, b) we move our node_modules search into core and it just does it. (The browser would need to have a separate script reference to the plugin, like it does now, if it was going to be loaded from node_modules, which may mean that plugins need unique identifiers? - [another decision needed]) So, basically, supporting current plugins via @plugin, which is one thing we agree on, yes?
  2. A @plugin call can search and "install" a "local" plugin, the way that @plugin works now
  3. In order to find middle ground, there may be some alteration that needs to happen in the implementation of either system. If so, it would likely be the @plugin implementation that needs to change, since it's not established in the docs, or other systems based on Less.
  4. We set up a repo generator that makes an end user / plugin implementer not have to worry about all that stuff.

@seven-phases-max @rjgotten How does that sit with you?

@matthew-dean
Copy link
Member

matthew-dean commented May 3, 2016

@seven-phases-max Specifically on point #1 above, if @plugin "clean-css"; would load /node_modules/less-plugin-clean-css/index.js in a Node environment, how do you propose making that try to retrieve /node_modules/less-plugin-clean-css/dist/less-plugin-clean-css.js in the browser? OR would browsers always need an explicit script path? And if so, even if they include said distribution script, how does @plugin "clean-css" know that that file is loaded?

@rjgotten
Copy link

rjgotten commented May 3, 2016

A @plugin call should be able to load an npm installed plugin if (decision needed) a) the Node plugin is loaded or, b) we move our node_modules search into core and it just does it. (The browser would need to have a separate script reference to the plugin, like it does now, if it was going to be loaded from node_modules, which may mean that plugins need unique identifiers? - [another decision needed]) So, basically, supporting current plugins via @plugin, which is one thing we agree on, yes?

Yes.

A @plugin call can search and "install" a "local" plugin, the way that @plugin works now

Yes; you don't want to lose the utility of having hassle-free, collision-free plugin functions being included via frameworks that users may incorporate into their own projects.

In order to find middle ground, there may be some alteration that needs to happen in the implementation of either system. If so, it would likely be the @plugin implementation that needs to change, since it's not established in the docs, or other systems based on Less.

That's reasonable.

We set up a repo generator that makes an end user / plugin implementer not have to worry about all that stuff.

If it can be automated succesfully, then that sounds like a good idea.


Technically our dispute is just about what @plugin "foo"; should search first...

If you want to combine local file references and global module references, then you have to be very careful with scoping and always ensure that, in the event of a name collision, there's always a way for the more specific local scope to overrule the global scope.

This is particularly important with Node.js modules, as the lookup can crawl a huge number of paths to find a potential matching name, including globally installed Node.js modules installed for all user accounts, over which the current user running the Lessc compiler may have zero control.

So here's my proposal:

  • For simple names that do not contain path separators, attempt to load as a global module first and revert to a local file if that fails.
  • For names that contain path separators, only attempt to load them as a local file. It makes no sense to provide explicit paths when loading a module, since modules should load by name only and allow Node's lookup algorithm to do its work.

If you have to load a plugin file that sits next to the less file that imports the plugin (and there is no path separator), then a user can explicitly request the local file by using the 'current directory' token, like so: @plugin "./local-plugin"

@rjgotten
Copy link

rjgotten commented May 3, 2016

Specifically on point #1 above, if @plugin "clean-css"; would load /node_modules/less-plugin-clean-css/index.js in a Node environment, how do you propose making that try to retrieve /node_modules/less-plugin-clean-css/dist/less-plugin-clean-css.js in the browser?

Given a URL to the module itself, employ the following process:

  1. Request the module's package.json file.
  2. Parse it and scan its config section for a browser-dist entry, which holds the (module-root relative) path to the browser distribution.
  3. Combine this relative path with the URL to the module and request the browser distribution.

The real trick is replicating the module lookup tho'...

@matthew-dean
Copy link
Member

matthew-dean commented May 3, 2016

This is particularly important with Node.js modules, as the lookup can crawl a huge number of paths to find a potential matching name, including globally installed Node.js modules installed for all user accounts, over which the current user running the Lessc compiler may have zero control.

If you're running the Less compiler in Node, can't you you just do require("less-plugin-clean-css") / require("clean-css") in a try / catch block? Why not just let Node do the work?

For the browser, grabbing a package.json file sounds like a good solution.

For names that contain path separators, only attempt to load them as a local file

Another great solution. I like the idea of specifying local-only with ./. I still feel like, if you are compiling your Less in-browser, local should be the first check. If you have installed via Node, it makes sense to search Node first. So the file manager adapters (extensions, whatever we call them) should decide the file resolution outside of Less core.

@matthew-dean
Copy link
Member

(@rjgotten - As a random aside, why aren't you on this list? https://github.com/orgs/less/people)

@rjgotten
Copy link

rjgotten commented May 4, 2016

If you're running the Less compiler in Node, can't you you just do require("less-plugin-clean-css") / require("clean-css") in a try / catch block? Why not just let Node do the work?

The point I was trying to make was not related to performance or complexity, but the fact that a module name could be matched with a lot of places that are potentially not under a user's control. So you should make sure users always have a way to bypass the global lookup. (This is especially important for framework authors that want to distribute plugin files with their less files, as they have zero control over the target environment, period.)

Hence my suggestion to use ./ as a marker to only search through local files.

However, to answer your question: yes, the module lookup probably could just use require("less-plugin-" + <modulename>) to do its work instead of needing to prepend layers of ../. The trick, then, is in using the right require function.

E.g. for plugins installed side-by-side with the compiler itself, you could just assign the require from the root lessc module to a property on the less object and make it reachable and usable from the plugin loader.


As a random aside, why aren't you on this list?

Honestly; I can't guarantee available time to commit to the Less project. Too much going on. So, I'd rather stay off the official lists.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 4, 2016

if @plugin "clean-css"; would load /node_modules/less-plugin-clean-css/index.js in a Node environment, how do you propose making that try to retrieve /node_modules/less-plugin-clean-css/dist/less-plugin-clean-css.js in the browser?

There's no point to use node dirs in a browser (it's a different "platform"), so browserified versions just have to be searched relative to the root of the current HTML document (or at some pluginPath specified via less options so you could point it to a CDN).

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 4, 2016

This is especially important for framework authors that want to distribute plugin files with their less files, as they have zero control over the target environment, period.

So I have another idea (a bit breaking but just to consider). Maybe then we just should stop calling my-functions.js a "plugin"? Because the way @rjgotten describes it does not look like something discussed at #1861, #1917 etc.. Essentially it's really just "a custom script" with a few predefined requirements (generic platformless-JS with no external dependencies and no access to anything of external world in general) (i.e. basically the inline version of http://lesscss.org/usage/#using-less-in-the-browser-functions). Then taking this into account, should not we switch it back to the originally proposed @import "my-functions.js"; (thus leaving @plugin for plugins) and never mix up "(Less) plugins" and "custom (Project) scripts"? And all contradictions pass away automatically, don't they?


This of course won't mean that certain my-stuff.js is forbidden to work as both (after all they still overlap a lot), the only reason is just to stop trying to join unjoinable things via the same entry.

@rjgotten
Copy link

rjgotten commented May 4, 2016

We originally separated the keyword from import to prevent confusion with less or css imports tho.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented May 4, 2016

We originally separated the keyword from import to prevent confusion with less or css imports tho.

Yes and no. First mentions of @plugin are less/less.js#1861 (comment) and less/less.js#2479 (comment).

And for reasons we both wrote above, @import "my-functions.js"; (and then -> @import (script) "my-functions";) is looking much more similar now to @import (less) "my-mixins"; than @plugin "clean-css-or-so;" could be.

@matthew-dean
Copy link
Member

matthew-dean commented May 4, 2016

Then taking this into account, should not we switch it back to the originally proposed @import "my-functions.js"; (thus leaving @plugin for plugins) and never mix up "(Less) plugins" and "custom (Project) scripts"? And all contradictions pass away automatically, don't they?

Ho boy. Hmm....

In a way, you're suggesting what I've been suggesting earlier, which was to have Plugin format A, and plugin format B, and have both exist in parallel. (Although the goal as I saw it was to move from A to B over time.)

I get your reasoning. In the case of something like clean-css, it's something that the author is applying to their final project as a whole. If you didn't have the plugin running, your code would be output just fine.

In contrast, with many @plugin examples, and with the original implementation, those "plugins" are functions that are direct dependencies of the .less code. They exist as pairs, which is part of the argument to have .js live where .less does, because they are essentially two parts to a whole. In those cases, .less + .js = .css.

The danger is, of course:

  • User confusion - "What the heck is this new thing? And which thing do I use/install?"
  • Developer confusion - "Which method do I use to solve my problem?"

That said.... we don't necessarily avoid user confusion by having two plugin "formats" that have different loading mechanisms, if we indeed still call them both "plugins". That may, in fact, be more confusing. True, we were hoping to make the formats the same, but they would still have two different loading mechanisms / rules.

The benefits might be:

  • All the work/headaches discussed involved with unifying them kinda goes away, as you say.

It's a radically different direction than where we've been going, so I'm glad you brought it up. I think.... that could work as long as we would clearly delineate use cases, as in:

  • Plugin: Installed into the core "Less engine", and so modifies Less functionality. Used to modify input / output of project on a global level (e.g. clean-css, autoprefixer)
  • Scripts ("helper scripts"? "helpers"?): Used when there is a direct dependency on additional functionality in your .less code and you want that functionality isolated to a particular scope/library (e.g. functions / custom @-rules, etc)

Now, both of those would have some possible overlap. And I still think the two could move a bit closer together (moving devs away from "node-only" plugins, allow adding visitors,etc. from "scripts")

The one thing I would disagree with is overloading @import. I think all the original reasons against that are still valid. Namely: @import's options right now have to do with the way those styles are interpreted. (AND as you correctly pointed out, we may want to add the ability to set options for a script.) So, to me, a logical change would be to @script "my-script"; (unless we think of a better @-rule name)

I think if we do that and separate the two, we also toss out all the complicated Node searching. It's just not worth the additional cognitive burden. We could still easily pass "scripts" to the initial Less options / setup that someone wanted available globally.

This is not a bad idea. It does seem a hell of a lot easier to implement, it's a bit more pragmatic, and may actually cause less confusion.

@rjgotten What are your thoughts?

@matthew-dean
Copy link
Member

matthew-dean commented May 4, 2016

Another thing about @helper/@helpers. While the script may contain something like helpers, there's really no requirement that it does, or that it returns anything at all. It's not returning a particular object, for example, that is added to a "helpers" collection or anything like that. The initial examples are custom functions, but, as you say, it may not be. So, I wonder if the same argument against @functions would actually apply to @helpers, since a helper is typically a specialized type of function. Dunno. @script may be the easiest to evolve, and mimics the genericness (and grammar convention) of @import and @plugin.

@matthew-dean
Copy link
Member

matthew-dean commented May 28, 2016

Some other options to put on the table here:

@component "foo";
@module "foo";
@require "foo"; // may semantically support the scoping behavior
@extension "foo";
@addon "foo";
@expansion "foo";

Would be nice to come towards an agreement on the name so we could start to document it.

@matthew-dean
Copy link
Member

matthew-dean commented Jun 23, 2016

@less/core Bump. I think we have a plan we just need to agree on a name.

@matthew-dean
Copy link
Member

I think my preference right now might be @require or @extension (since it's typically a "companion" to .less code within a scope), followed by @script.

@matthew-dean
Copy link
Member

A dev friend of mine just suggested @snippet.

@amatecha
Copy link

I'm a fan of @require, pretty universal meaning IMO - some kind of "import" that is required for your stuff to work 👍

A couple others to throw out there:

@annotation
@using

@amatecha
Copy link

@matthew-dean suggested @use -- "Use in this context". Pretty concise and conveys the intent, even if slightly ambiguous. I'm a fan :)

@zenimpulse
Copy link

Second on @use. Concise, and prevents confusion with JS conventions (require) and existing LESS methods (extend).

@matthew-dean
Copy link
Member

Yep, I prefer it to something like @helpers because it's an action like @import (and even semantically similar), without needing to specify what the script consists of that you're using, and not so generic like @script.

3 points for Griffyndor. I mean, @use.

@rjgotten
Copy link

Yep, I prefer it to something like @helpers because it's an action like @import (and even semantically similar), without needing to specify what the script consists of that you're using, and not so generic like @script.

Took the words out of my mouth.

@matthew-dean
Copy link
Member

@rjgotten Is that a +1 then?

@rjgotten
Copy link

@matthew-dean
Yup.

@matthew-dean
Copy link
Member

matthew-dean commented Jul 2, 2016

Soooooo........ @rjgotten @seven-phases-max

Here's a little progress update.

TL;DR: I figured out that the two formats actually needed to be unified after all, and went ahead and did the work.

Detailed Explanation

I started work on updating @plugin to @use, and then started to test it with a project. If you recall a while ago, I submitted a PR proposal for interpreting custom at-rules as functions. @seven-phases-max's suggestion was that this was handled by a plugin that added the appropriate visitors. This was fine by me, so I started developing this with the inline syntax.

However.... I began to run into some problems. The right vars / objects weren't exposed to the inline scripts, so those pieces from the PluginManager needed to be exposed. Then, I realized there were some use cases that couldn't really be handled by a script running once and then exiting. The @plugin/@use statements needed to support re-use. In other words, they needed to expose functions on a plugin object. You know, functions like install() and possibly a few others. (Meaning these "extensions" were beginning to look pretty much identical to "plugins".)

Meanwhile, I came across the plugin loader used by lessc. It handles retrieving a plugin by module name, and does a lot of convenient things like checking the version #, trying to prepend "less-plugin-" if no match is found, and if a plugin returns a function instead of an object, it attempts to create a new object out of it, which the Less.js API itself doesn't do, but should.

All of that led me to the inevitable conclusion that @seven-phases-max's instincts were right all along, that really, the historical "function script" @plugin format needed to align with documented plugin syntax, rather than creating another "format" which is actually not distinct enough to make sense.

Once I had the idea that the two formats might need to be unified after all, I obsessively programmed to see if this was a possibility, and eventually I had a branch with these changes.

  1. Both formats have a single backwards-compatible format, by:

    a. Evaluating less Node plugins within the function context @rjgotten created for @plugin.

    b. Adding either file type to the plugin manager

    c. Allowing an evaluated plugin file to either assign to module.exports = { ... } or return { ... } to return an object with functions. (Or returning a function which, when invoked, returns an object.)

    d. Conditionally evaluating the install() method, only if it exists.

    e. Using the plugin loader method for lessc as a "platform-specific" loader for @plugin imports.

    f. Adding a "local path" option to that loader, so that the plugin loader for Node will also look locally (local to the @plugin reference), if @plugin is what invoked it.
  2. I changed the visitor collection in the plugin manager to be a dynamic iterator. Meaning, if visitors are added during visiting, then those will still get visited. I did this to support the use case where @plugin was invoked within a ruleset. (I noticed while testing that visitors added "late" would never be visited.) This still makes visitors global, but I'm not sure if that's an issue.

Based on initial testing, all of this works. I was able to do this:

@plugin "inline-urls";   // loads less-plugin-inline-urls module installed via 'npm install';
@plugin "local-file";   // loads local-file.js located in the same folder as the file where this was invoked

Note that the behavior of Node module loading is handled by the less-node plugin manager. I still need to test the less-browser plugin loader, which would only support local files. (Anything else would be too expensive in the browser, and it doesn't make sense to support Node in browser mode.)

Final Thoughts:
After seeing that all could be made possible under @plugin, I realized there was really no need to have to explicitly load a plugin under any circumstance (via passing to Less options). You don't have to require() a plugin and pass an object. @plugin is a cross-platform declaration, so it makes things a ton easier when explaining it.

I still have some more work to do to:

  1. Add / test browser support
  2. Add tests?
  3. Fix thrown exceptions. Those are still a bit awkward when there's an exception in the plugin file. The returned LessError is not quite accurate.
  4. Maybe having a look at the Less tree API. But that could be for 3.x.

I added these changes to the feature/@plugin branch and started a new 3.x branch, but, as I don't think there are any breaking changes, it could come in before 3.x.

Feedback welcome.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented Jul 2, 2016

👍


Btw., indeed re-reading posts above I can see now that probably we digged too deep into "where plugin is loaded from" thing, which is in general quite subtle issue and won't be worth that possible overengineered @plugin/@use stuff while not really targeting the main thing (i.e. let the Less be as extensible as possible in an as easy as possible fashion).

@matthew-dean
Copy link
Member

@seven-phases-max Yes, I realized that too. It's actually quite a minor point. On Node the file searching order is not that relevant and really not that expensive. And I think I got stuck on having similar behavior on two different platforms, which is also not necessary. The compiling environments are very different, and it's easy to say, "On Node, Less will search node_modules first. In the browser, it won't." My plan is to have the browser loader do normal @import behavior, with the caveat of trying to prepend "less-plugin-" as a second try. With the exception that having ".js" just looks locally, as you first proposed. Basically, you had it right the first time, lol.

Once I got into the codebase and started digging/coding around plugins, the way you had described it made more sense. The tricky part was to fold in existing @plugin behavior, which we didn't necessarily have to do, but I liked how much it simplified things the way that @rjgotten had done it.

Meaning: as I currently have it written, a plugin doesn't have to export anything. It can add one function when it's first evaluated and be done (since the "functions" registry is now passed into the scope, as long as @plugin is the caller). So it makes the plugin "signature" optional (albeit recommended).

I have to do some work yet to fix lessc to use the new plugin loader, and setting up the browser, but should be good to go before too long.

If you have a chance to look and make any suggestions, please do.

@matthew-dean matthew-dean mentioned this issue Jul 2, 2016
31 tasks
@rjgotten
Copy link

rjgotten commented Jul 4, 2016

Sounds good. 👍

@matthew-dean
Copy link
Member

I'm recommending that we do a 3.0 release with these plugin changes sooner rather than later. I've disabled inline JavaScript along with the plugin changes because of a security report that was sent to me. Talk to me offline if you want more details.

@matthew-dean
Copy link
Member

Documentation well on its way! https://github.com/less/less-docs/blob/3.x/content/features/plugins.md

@matthew-dean
Copy link
Member

Update on @plugin

I realized in the process of documenting plugin usage (which I now realize is a good practice for figuring out if intended usage actually makes sense), that there were problems with a few specifics in the "format" of the file.

Namely: having globals like less and functions available, or ending with a return statement created add-on problems, like:

  1. Linters immediately complain, and rightly so, because the file is essentially a JavaScript "fragment", to be later constituted.
  2. It limits / changes where / when in the Less.js lifecycle a plugin can be added. For instance, it can longer be included in a <script> tag as is currently documented for pre-loading plugins.

So, what I've decided to do (but let me know if there's any opposition) is to recommend in the documentation specific boilerplate code that's essentially UMD format. (See the recently updated: https://github.com/less/less-docs/blob/3.x/content/features/plugins.md)

I can (and probably will) still leave less, tree, and functions available as globals for any legacy @plugin v2.5.0 files that may exist, although there shouldn't be a lot due to lack of documentation. These globals will be replaced by local vars of the same name (and the same objects) when using the boilerplate UMD code (except for tree, but a reference to tree is no longer necessary), so it shouldn't be a problem.

Once again, if anyone objects, let me know, but the boilerplate code should allow plugins to be loaded in Node.js, either via require() or via lessc or via @plugin, as well as loading in the browser via @plugin or in a <script> tag.

@matthew-dean
Copy link
Member

What would help me is if someone could get repos set up with cross-platform UMD plugins, to replace:

I'm not sure of a use case for a cross-platform file-manager. A plugin that connected to the Dropbox API? Seems silly in the browser. But the point is just to get a few plugin examples that are universal, so the use case doesn't matter that much.

If someone can also come up with a repo layout like:

- src/    // can be directly "require'd()"
  - index.js
  - otherStuff.js
  - UMD-wrapper.js    // wraps module in boilerplate for browser build
myplugin.js   // builds for browser
myplugin.min.js
gulpfile.js  // makes the above browser builds
package.json // has necessary includes for building a plugin

That keeps plugins simple & flat, and can keep source files organized while outputting single-file builds.

@rjgotten
Copy link

rjgotten commented Jan 9, 2017

@matthew-dean
I'm all for the UMD pattern. Makes everything nice and explicit. However, I'd rename registerPlugin in your documentation to a more simple factory. That particular function doesn't actually perform any registering of the plugin, it only creates the plugin object. Less itself does the actual registering by calling the install member.

It's kind of sad that the browser-globals based version bloats the UMD config because of a need to detect prior existence of the global less object and the less.plugins array configuration. I wonder if you could shorten it. Possibly something like:

var ns = root;
ns = ns.less = ns.less || {};
ns = ns.plugins = ns.plugins || {};
ns.push(factory());

Or maybe just forego the two-tier namespace altogether and just use a global less_plugins array that Less can detect in addition to the properly namespaced less.plugins. That would allow you to turn it into a one-liner, such as:

(root.less_plugins = root.less_plugins || []).push(factory());

Also, my gut tells me the install method is better with the parameter order altered to place pluginManager last. The less and functions members are probably going to be the most used, which will allow plugin authors to omit the more complex pluginManager from implementations that don't need it.

In total:

(function (root, factory) { 
    if (typeof define === 'function' && define.amd) { 
        define([], factory);
    } else if (typeof module === 'object' && module.exports) { 
        module.exports = factory();
    } else { 
        (root.less_plugins = root.less_plugins || []).push(factory());
    } 
}(this, function () {
    // Less.js Plugin object
    return {
        install: function(less, functions, pluginManager) {
            // functions.add('')
        }
    };
}));

@matthew-dean
Copy link
Member

matthew-dean commented Jan 9, 2017

Also, my gut tells me the install method is better with the parameter order altered to place pluginManager last.

Ideally, yes, but that would break existing plugins. The function registry is something I added, otherwise there's no "local" function registry passed to install, as it was previously just a global var.


Renaming to factory() is fine. It's an arbitrary name.

Yes, you are right that I was trying to make it so Less config could appear before or after plugin <script> tags, if someone was including a plugin that way. In fact, I was hoping to simplify the documentation so that adding a plugin didn't mean creating a Less object at all (if you weren't adding any other configuration).

So, yeah, I could make it another global to shorten it, but I would capitalize LESS_PLUGINS for an internally-used global. That's just the naming convention I've used.

Update: actually, you're right on another count. It does need to be a second global, because otherwise you'll have people do this:

<script src="first-plugin.js"></script>
<script src="second-plugin.js"></script>
<script>
less = {
  plugin: [secondplugin]
};
<script src="less.min.js"></script>

@matthew-dean
Copy link
Member

Also, to be more accurate, functions is really (local) functionRegistry, but I did that for backwards-compatibility / simplicity.

@matthew-dean
Copy link
Member

@matthew-dean
Copy link
Member

Should this discussion be closed for cleanup?

@seven-phases-max
Copy link
Member Author

Closing as implemented in v3 (for various feature spin-offs mentioned during the discussion please create dedicated tickets).

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

5 participants