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

New import option node to use require.resolve logic #2618

Closed
JedWatson opened this issue Jun 22, 2015 · 23 comments
Closed

New import option node to use require.resolve logic #2618

JedWatson opened this issue Jun 22, 2015 · 23 comments
Labels

Comments

@JedWatson
Copy link

This is a spin-off from the conversation in #2560.

As we start publishing more front-end components on npm (in my case specifically, react.js components) less would provide a great way to bundle customisable CSS alongside the javascript code, that can be provided as a built file, or as a less file supporting variables for themeing, etc.

However there's a critical problem in doing so, because it's impossible to know where the actual path of the module is without replicating node's require.resolve logic, or using the npm-import plugin.

For clarity; if I depend on react-select in keystone with a require path like ../../node_modules/react-select/less/component.less, it works until the project that depends on keystone also depends on react-select, at which point the path becomes ../../../node_modules/react-select/less/component.less and my import statement breaks.

You can see my workaround here.

I propose a new import option, node, that would work like this:

@import (node) "react-select/less/component.less";

The node option would use the require.resolve function in node.js / io.js to get the path to the react-select module, then prepend that to the rest of the specified path. In my case above, it would be expanded to the equivalent of:

@import "../../node_modules/react-select/less/component.less";

For context, we're working on a React.js UI Component Library called Elemental UI which is composed of discrete, independently packaged UI components (e.g react-date-select), and using that in KeystoneJS at which point things get really crazy without this feature.

For the moment the npm-import LESS plugin is a workaround, as is the logic I've written into Keystone, but this would really be much clearer as a language feature and I'm not particularly comfortable enforcing that plugin on our audience; I believe plugins should be included at the discretion of a project author, not because some package (potentially several packages deep) in the dependency graph requires them.

@seven-phases-max
Copy link
Member

update

(just to make it more evident to what of two request here I'm answering below).
To make thing a bit more clear, there're two pretty much unrelated requests above:

  1. An option for the less-plugin-npm-import to translate url paths according to the imported package own location.
  2. A request to build less-plugin-npm-import into the Less core.

Obviously 1. is a subject for https://github.com/less/less-plugin-npm-import/issues
(though see #2615 I'll mention in later comment). And below goes a comment for 2. only:

~ end of update


See discussion for "plugin or core" in #1972.

I'm not particularly comfortable enforcing that plugin on our audience;

For "plugin-fear" discussion see for example #2564 (comment).
After all you do enforce Less (not just "a Less compiler" but "an up-to-date only less.js only implementation"). More over you do enforce node, npm and whatever other stuff they bring! So another question is: if I can use Elemental UI only via node and npm how using npm-import plugin can harm? Or, if I can use the library w/o node how "a built-in npm-import" could help?

The desire to make things more simple for you to maintain is understandable, but please take into account an opposite vision: someone else do not want to enforce compiler burden and not-directly CSS related features (always remember Less is nothing but just a CSS+) with all of their extra-code on people who won't use these external to the language stuff.


Aside of above, there was already an (quite obvious) idea of having a fork with all common stuff (useful plugins and/or even Less libs of mixins/templates) included by default (e.g. lessc-ext, less-ext.js or so). Eventually it will happen most likely, and I'm afraid this is the only way to reconcile "keep it minimal" and "put everything in" approaches.

@dcousens
Copy link

I believe plugins should be included at the discretion of a project author, not because some package (potentially several packages deep) in the dependency graph requires them.

This is also my concern with the resolution given for #2560.

The only way the top level user can actually use some library's less is to be aware that [potentially] some sub-dependency (which could be many layers deep) might require --relative-urls to work correctly.

For clarity; if I depend on react-select in keystone with a require path like ../../node_modules/react-select/less/component.less, it works until the project that depends on keystone also depends on react-select, at which point the path becomes ../../../node_modules/react-select/less/component.less and my import statement breaks.

You also have the case of npm dedupe, which will give similar breaking behaviour, this is because, in both cases, the package goes to a higher level in the dependency tree.

@dcousens
Copy link

This issue is related to #1972, but the arguments presented there are mostly a discussion around separation of concerns in terms of the role of less.js (AFAIK).
While relevant, that isn't directly related to the issues presented here which are around the module-wise composability limitations of less in its current state, plugin or otherwise.

@seven-phases-max
Copy link
Member

Just in case referencing #2615 here as pretty much related (just not bounded to npm or any other tool/library-releasing-format).

@rjgotten
Copy link
Contributor

The only way the top level user can actually use some library's less is to be aware that [potentially] some sub-dependency (which could be many layers deep) might require --relative-urls to work correctly.

FYI: There's not directly an issue there.

The author of said sub-dependency should be checking whether the setting related to relative urls is set to true on the eval context. If not, the dependency should probably throw a correctly formatted error object and the Less compiler will then report that.

The only issue would be if the sub-dependency required relative urls and some other part required non-relative urls, leading to a conflict.

@matthew-dean
Copy link
Member

-1

Less 2.0 was designed to be largely platform agnostic. Specific environmental features are what the plugin system was designed for.

And also this:

someone else do not want to enforce compiler burden and not-directly CSS related features (always remember Less is nothing but just a CSS+) with all of their extra-code on people who won't use these external to the language stuff.

Recommend closing.

@JedWatson
Copy link
Author

@matthew-dean please don't close yet, I've been writing up my reply to @seven-phases-max and just haven't had a chance to finish + comment here yet. Give me a few hours to come back to this, I'm flat out at work right now.

@dcousens
Copy link

FYI: There's not directly an issue there.

Then

The only issue would be if the sub-dependency required relative urls and some other part required non-relative urls, leading to a conflict.

I'm confused, this looks like an issue to me if we're trying to build a composable ecosystem?
This is [one of] the underpinning issues I am trying to expose.

@JedWatson
Copy link
Author

Thanks for the replies @seven-phases-max and @matthew-dean. Apologies in advance for the length of this comment; I think this is really important and am doing my best to explain why.

TL;DR

  • CSS packages could be awesome
  • @import is currently incompatible with CSS packages; I believe fixing it in core is important
  • We also need to fix it to enable packages of composable components
  • Plugins are great for projects, not packages
  • Plugins are not documented / supported everywhere
  • As most client-side tooling is in node (grunt, gulp, webpack, browserify, babel, etc) and npm is becoming the dominant package manager for the client as well as node, I believe node package support is not as environment-specific as it sounds

First off, I obviously missed the prior conversation and @ForbesLindesay's PR (which does exactly what I'm suggesting, except that the option is named npm instead of node). So, sorry for recreating the issue without having the background.

However, that is my first point - I have actually spent a lot of time with the LESS documentation, raised a similar issue, and understood the source code well enough to use the undocumented modifyVars syntax (documented for CLI, not API usage) in my Keystone workaround. Yet I hadn't found the plugin before @dcousens comment on #2560. I think that the contrast between this project with 12k+ stars, and the plugin's 15 (inc. me now) tells the story more clearly than I could here.

And while we could certainly raise awareness about plugins through updates to the site and docs, that only sort-of solves my concern; the way build systems and libraries are progressing and particularly with the increasing awareness of node-style require patterns on the client side (thanks hugely to browserify and webpack, but I would also credit babel and React for getting everybody in that ecosystem to get their heads around transpilers), I believe it's highly beneficial to enable an ecosystem of npm packages that provide LESS.

I actually foresee, e.g with resets or grids, npm packages comprised entirely of css or mixins. Imagine if you could npm install bootstrap-grid, which then depended on bootstrap-mixins. You could have packages that bundle animations, transforms, all of which could be configurable with variables and used in both projects and other packages.

The problem is that if it's not a first-class part of LESS, it's just not going to happen and we'll miss out on that opportunity (again; in my opinion.)

To be clear here, I'm not looking for something that simplifies @import "../../node_modules/... into @import (node) ...; I am talking about bringing node's hugely successful package management and path resolution systems to CSS. Across several projects I'm involved in, we're trying to work out a realistic way to deal with composable component packages bundling css (with variables) and javascript modules. This feature is the missing link.


Consider the following:

project
    packages
        mixinsPackage
        gridPackage
        animationsPackage
        resetPackage
        uiLibraryPackage
            packages
                mixinsPackage
                animationsPackage
                datepickerPackage
                    packages
                        animationsPackage
                        buttonsPackage
                buttonsPackage
                inputsPackage
            styles
                defaultTheme
                stylesheet
    styles
        theme
        stylesheet

Imagine I have pulled several generic packages that I want to use in my project: mixing, a grid system, animations and resets. I have also included a UI Library.

The UI Library happens to use the same mixins and animations, and includes its own subpackages for a date picker, buttons and inputs. The date picker also uses the buttons package, and animations.

Now imagine the UI Library deus variables that the Buttons (and other) packages will use. It overrides the defaults in those packages. And imagine I can reset some of those variables in my project's theme file. This is how you could cascade themes through packages that are composable and also usable in isolation.

I have a solution for this at the component level. The current alternative is to put the styles in my javascript, which is what other major UI Libraries have done; I'm not ready to give up the stylesheet yet though (for several reasons) and I'm hoping to solve the problem using LESS.

For the example above, my main stylesheet would look something like this:

@import (node, reference) "mixinsPackage";
@import (node) "gridPackage";
@import (node) "animationsPackage";
@import (node) "resetPackage";
@import (node) "uiLibraryPackage";
@import "theme"; // this resets variables used in uiLibraryPackage

// my classes or other includes using the mixins defined above go here

I realise this does not completely solve all the problems. Packages still have to use namespaces properly to avoid conflicts, etc. But at least it makes it possible.


re "plugin fear" remember that not everybody uses LESS the same way (ref my difficulties using modifyVars). Even if plugin usage is documented for the command line and API, how do I include it in my gulp file? webpack? etc. Those integrations are not documented on the less website; they're in other sites, or GitHub repos, and not consistent. For example, less-middleware makes no mention of support for plugins at all; if it's in there, it's undocumented. I'd have to read through the source to work it out.

There's no way around the barrier to entry for plugins being much higher.

Also on plugins; I happily use babel plugins in my packages, because I control the build process. Although I write ES6 with JSX, I ship vanilla ES5. So nobody using my packages has to know or worry about which plugins (or which versions of which plugins) I'm depending on. In that situation, depending on plugins in packages is fine. However the nature of CSS means that's not an option; anyone wanting to customise the look of my components with their own variables has to run less themselves; which means I'd be forcing my users to understand my plugins, and hope they work in their environment.

My previous experience supporting both webpack and browserify and needing plugins (before I started just shipping transpiled code) has taught me this is a nightmare.


After all you do enforce Less (not just "a Less compiler" but "an up-to-date only less.js only implementation"). More over you do enforce node, npm and whatever other stuff they bring!

Yes I do enforce LESS. Honestly, if I could easily also support SASS (like Bootstrap has) I would, because being less prescriptive means more people can benefit from my work. But since I have to pick, LESS is my favourite, and I'm happy to promote it.

The rest are simpler; For Elemental specifically, it's a React library, which means you already probably need node, babel, npm, and a build process. These requirements are enforced by bigger pieces of the ecosystem, and I am ok with that.


I do get the argument about not bloating core with really specific features. I think #1972 was mis-named, and I originally nearly called the option npm in this issue too, but everyone in that issue got caught up in the package manager aspect; these aren't "npm modules", they're "node modules" and specifically I'm just advocating here to be able to use "node-style path resolution" in an import statement.

If you think of this as a core way to solve the "css packages" issue, it becomes less of an environment specific addition and more of a powerful option for people running LESS in a node environment (obviously, this would not be supported in the browser version, but that is also not a production solution). I've seen php, ruby and python apps all include a package.json and node packages to support their client-side build process, using gulp, grunt, etc. As the popularity of tools like webpack and babel increases, so will this.

So to wrap this up:

someone else do not want to enforce compiler burden and not-directly CSS related features

Based on my review of #1972, aside from including two additional packages if you install LESS in node, it's not much of a burden to give this first class support. Sounds like we're discussing a principle here, that this addition is perceived as being against.

The @import statement is a css related feature, but it's been thought through from the client-side perspective. In a "compile on the server" sense, if you're including css from packages, it's actually just broken. This proposal fixes it, so I can nominate my import as coming from a package instead of a relative path.

Thanks :)

@seven-phases-max
Copy link
Member

seven-phases-max commented Jun 23, 2015

Well, I already feel like I start to repeat the same idea again and again and again just using different words. So to save our time: Before I sound the idea itself it's worth reading this thread (just as an example of many more), otherwise the following may sound too harsh.
See? That's the axiom: Everybody want everything in the core. Everybody find the feature they're interested in to be the most important. Everybody believes his feature is an exception of that everything... But in the same time nobody is going to actually support/maintain this everything (I'm not even talking about documenting it). (This of course is no way to offense this particular FR or you personally, it's just a generic observation to make the state of the things more self-evident). So it's obvious and expected the people who are not ready to give Less up because the implementation becomes unmaintanable-everything-monster will always resist any "to the core!" stuff until the feature starts to actually be that unavoidable in the core thing.
So I'd suggest to try think of plugins from the opposite corner: what if the real choice is "support via plugin" or "no support at all"? (actually for many things it's more close to truth...)
Or maybe even in simple words: no matter how difficult or undocumented it may look now, but looking for excuses to reject plugins is actually a road to stall any further Less development (sure it's sort of flawed dependency, "plugins can't become better if nobody uses them, and nobody uses them until they are better" - but still it's better then just nothing as in void).
These are the main reasons why when in "core or plugin" doubt the choice is always "plugin"... I.e. it's not really about convincing how useful it could be or so but more about how future-proof it is.
I.e., getting back to the less-plugin-npm-import itself, If it becomes more tedious to maintain it as a plugin (for example by continuously answering for "how to use it with ***") than maintaining it in the core, then the story most likely changes.

Well, I guess you've got the idea.


@JedWatson I have not yet read everything, so just a few comments (a bit muddle maybe - I'll fix later):

Even if plugin usage is documented for the command line and API, how do I include it in my gulp file?

Same way you include other things there. E.g. package.json -> "dependencies" -> "less-plugin-npm-import" then as in "Using Plugins". No extra efforts for end-users at all.

webpack?

Assuming it can work with gulp and grunt I guess there're some ways...

CSS packages could be awesome

Probably, if there're tools to move/translate your paths/files into "release state" to be understand by a server/browser. So the proposal wants Less compiler to become such tool for less files? Sound reasonable but:

(grunt, gulp, webpack, browserify, babel, etc) and npm is becoming the dominant package manager

The key difference between these tools and the Less library (i.e. "why Less can't and should not become such node bounded") is that (most of) those tools are (roughly speaking) sort of "package managers" themselves, i.e. "collecting and using different node packages" is their core functionality (not always the only of course). In the same time the core functionality of less.js is (well, yep, obviously) is to translate one language to another, and while certain interoperability with "external world / environment it works in" is somewhat unavoidable, a compiler can't and shouldn't become such a package manager.

I think that the contrast between this project with 12k+ stars, and the plugin's 15 (inc. me now) tells the story more clearly than I could here.

Star counting is a dangerous game. Beside considering that less is 6 years old and its plugin facility is only about .5, this may in fact illustrate the opposite thing: nobody is interested in the feature ;) (I'm half-joking of course, but you've got the idea - it's just almost always useless to compare stars or downloads or anything like that for different stuff w/o "how and where it's to be used" context (I saw some people still using browser-side less.js script v1.1.3 released five years ago, do we count their stars for Less in this case? :)

I ship vanilla ES5.

Yep, except "vanilla ES5" does not have any require, so before these scripts can reach a browser they need some extra tools or plugins to resolve it. But never mind (just a remark for wrong analogy, the analogy for "vanilla (browser compatible) ES5" would be "vanilla CSS". And since Less is a bit more complex thing than that (otherwise you'd ship just the CSS, right?) it is also expectable the corresponding tool-chain to be a bit more complex).

For example, less-middleware makes no mention of support for plugins at all;

Too bad for it. Considering Express JS (and similar stuff) can use node modules directly this only means that more people will give less-middleware up. But not yet a reason to put more duties on the Less team (more on this below).

@matthew-dean
Copy link
Member

To echo @seven-phases-max, it's not that this request doesn't have any perceivable value, but it seems to fall out of scope. It's not clear that this is the only way to create Less components. Any system could be built which delivered Less files that were imported / available via a Less plugin. I'm not convinced that there's something obviously missing or something that needs to be fixed. The case you make is basically if you approach the problem in a particular way and choose a particular path to solve that particular problem, only then do you encounter an issue with that approach.

That, in itself, doesn't necessarily mean that a feature isn't worth exploring, if it seems that it's a common scenario. But in this case, it feels like the "missing piece" is largely a problem created by the approach.

As far as node-style imports, I believe you can already set the "root" path for imports, and essentially prepend that path to import requests. Or you could set a variable that altered the import path. Or you could have a scoped plugin which you call in order to give node-style paths. There are many options here.

Plugins have really only just arrived, and no one yet has had the focus to really give them ample documentation. There are systems that will adequately and nicely support plugins, and I would anticipate supporting documentation that will make them perfectly approachable for end-users. I disagree they make more sense for projects then packages. There are design strategies which make them much more appropriate in packages / libraries, and I think we'll see good examples of those in the next 6-12 months.

@dcousens
Copy link

In the same time the core functionality of less.js is (well, yep, obviously) is to translate one language to another, and while certain interoperability with "external world / environment it works in" is somewhat unavoidable, a compiler can't and shouldn't become such a package manager.

Except, that without plugins, it can't perform that task correctly in modern build systems (that is, IMHO, a npm-like environment).
Granted, I can understand the skepticism you may have if you aren't confronted by these problems on a daily basis, especially since this an environment which feels like just about everyone is throwing spaghetti at a wall to see what sticks.

However, I don't think just "sitting back" and waiting 6-12 months is the right solution either, as that will just keep less from evolving with the community, and you'll come into a situation where some people will move away (the actual statistics of which are pointless speculating over).

@seven-phases-max
Copy link
Member

@dcousens

However, I don't think just "sitting back" and waiting 6-12 months is the right solution either

Your misassumption is that you think there's a choice between "waiting 6-12 months" and "something else". Well, look at the query to get the idea of why it's wrong.

So... Not happy with plugins? This is sad, but this is only thing we can offer for now. So it's primarily _your_ interest to help to make the plugin subsystem improved and wide-spread. Otherwise (_half-joking mode on_:) it's not a big deal to put "ReadyForImplementation" label for this FR and get it buried with other FRs within the query I linked above. (And it's not even about asking you or @JedWatson to make an initial PR, count how often I used "maintain" and "support" words in my previous post for another hint).

you'll come into a situation where some people will move away

Yep, yet another common convincing argument and yet another common misassumtion that any of active Less contributors get some direct benefits from the number of Less users...

@JedWatson
Copy link
Author

Trying to get back to something more constructive :)

I understand how much time it's costing you guys just to read and respond to this issue, so please know I appreciate it.

It looks like webpack does support less plugins. I still think it's hard to document, for each component, how it needs to be used other than "you need less.js v2.8+" if we have to point at plugins, but let's leave that for now.

The other plugins listed in the "preprocessor / feature plugins" (other than nom-import) are legitimately optional things that should be up to the end user. For example clean-css, Autoprefixer, etc. They are all environment specific and at the discretion of whoever is actually using the generated css.

The reason I believe the npm-import plugin doesn't belong in this group is that if someone includes my react-date-select less file, and it contains an import statement using the npm protocol, their compiler will break. It's an inversion of control. Is that an acceptable line to draw when deciding what should be supported in core vs. what should be a plugin?

Also conceptually, I think the fact that you can use node-style imports from packages belongs with the rest of the @import documentation. Even if it's an optional feature.

this may in fact illustrate the opposite thing: nobody is interested in the feature

I know you were sort of joking but I think the issue is the obscurity, not the lack of interest. Sample size of 1 here but I went looking in the docs and didn't find it, and even wrote my own workaround :(

why Less can't and should not become such node bounded

I was under the impression there is already a node-specific build, that could contain the @import (node) directive...? it's just enhancing the language for one of the most common platforms it is run on. In the counter example, there's browser specific API too (e.g resetting variables). How is this different?

"vanilla ES5" does not have any require, so before these scripts can reach a browser they need some extra tools or plugins to resolve it

Sorry, I didn't word it well. I actually ship ES5 CommonJS modules. They have require, and can be built with browserify, webpack, or run in node. My point was I don't ship ES6 or JSX anymore because that required plugins for those different systems and supporting it was a nightmare. I now do the things that require plugins (like babel) in my own build process and ship the result. I know you're concerned about having more to support in LESS, but I am speaking from experience here as well.

Unfortunately we don't have that option here; My whole problem is that, when I write an @import statement, I have absolutely no idea where the file will actually be in the filesystem when the LESS is compiled.

looking for excuses to reject plugins is actually a road to stall any further Less development

I've read #1917 and completely agree with your decision there. Pre-processors should be left up to the developer who's generating the CSS. Not everybody wants them, and it would add unnecessary bloat to the core and overhead for the LESS team.

Please believe I do not want everything in the core. And this is obviously a difficult argument to make, why my feature should be different.

I'm such a fan of plugins, I am actually the author of one of the most popular Babel ones.

Plugins are great for optional, end-user functionality. And if this were anything else, I could even just implement a pre-processor into my own build process. But again, I have no way of knowing where the file is without the node.resolve logic being executed at final-build time.

If it becomes more tedious to maintain it as a plugin (for example by continuously answering for "how to use it with ***") than maintaining it in the core, then the story most likely changes.

My concern here is that you may not realise it. It seems you don't believe my experience to say that publishing components that require plugins to build is a nightmare; I know this because I publish some of the most popular components in the React.js ecosystem, and have dealt with all the issues that get opened on my projects when build processes fail for that reason. I don't know if issues were also opened on webpack's repo, but I doubt they know about the issues I've had either.

@matthew-dean out of curiosity:

I'm not convinced that there's something obviously missing or something that needs to be fixed.

... is this actually a thing you've tried to do? working with nested packages containing LESS. If not I can understand why this might seem out of scope.

The case you make is basically if you approach the problem in a particular way and choose a particular path to solve that particular problem, only then do you encounter an issue with that approach.

in this case, it feels like the "missing piece" is largely a problem created by the approach.

Sure, that's fair enough. We have spent a lot of time trying to figure out how to approach the problem of shipping composable react components bundles with stylesheets that contain variables and share mixins. Looking at the code / structure example in my previous comment, if anyone else has a better idea it would be very welcome.

I would add that we're basing our approach on node's, which has been pretty universally praised.

As far as node-style imports, I believe you can already set the "root" path for imports, and essentially prepend that path to import requests. Or you could set a variable that altered the import path.

I don't think you understand the issue. None of these are workable solutions. The import may be in ../node_modules/package/stylesheet.less OR ../../node_modules/package/stylesheet.less depending on whether the package is a dependency of the top-level project. There's no way to know this, and no consistency between packages so a single "root" path won't work.

Also that would be enforcing an opinionated setting on the stylesheet author (the user of the package) - they have to set the "root" path to something specific. That option should really be up to them to control, they may have another use for it.

And it's not even about asking you or @JedWatson to make an initial PR, count how often I used "maintain" and "support" words in my previous post for another hint

I get it. If you don't believe me, look at my public commit history. I know how much work it is to maintain projects, and have felt the pain of letting features into core that should be plugins.

But let's talk about the specific overhead required to support this. From my understanding, #1972 is basically a complete implementation. It's approx. 30 lines of isolated code, implementing the importOption. All the actual logic is handled by the resolve package, which is maintained by substack and hugely popular. Does this actually present a core burden to LESS going forward?

I'm happy to submit a new PR and take on maintenance / documentation / etc for this. But if you think it's really going to be a problem supporting and maintaining going forward then I'll defer to your experience and drop it.

@lukeapage
Copy link
Member

Im not 100% happy but i for one would be ok with less node depending on and including the npm import plugin by default (possibly as an optional dep.).

It keeps the code separate but also there is little harm adding it since it requires use of a special protocol.

My only concern is whether it has options and how those are injected. We could only include it if it isnt already there, so it can be overridden.

That being said i do not understand why it causes so many problems to say your library requires less version x and plugin y.

Wdyt team?

@matthew-dean
Copy link
Member

That being said i do not understand why it causes so many problems to say your library requires less version x and plugin y.

To a large degree, the inline @plugin syntax solves this, if your JS file is within your library files, but we desperately need documentation to illustrate the differences between the two. It's something I've been wanting to write up, just as soon as that time manifests itself. If NPM was involved, it would make more sense that the library simply had other modules as dependencies, and those were called by the root plugin. In that scenario, it would make sense to do @plugin (node) "my-module" within your library file to call a node import, rather than @import (node) "my-module-url\mymodule" to call needed less files from a node module. I recognize there are some things inline plugins can't do that pre-loaded plugins can do, but for something custom like this, I think most of us are leaning towards a plugin-based solution. If your library is node based, they still would need to "install" the library. Any Less library you still have to "install" in some form, and you need a requirement of both Less + library. It sounds like what @JedWatson is saying is that connecting a node-based component system plugin to Less takes Less + Less node plugin + node library. But it seems perfectly feasible to do Less + node library (w/ Less node plugin dependency, installed when you npm install xyz plugin).

Which sort of brings me back full circle. And I agree, @lukeapage, I don't see how, if you library is node-based, that asking a user to install that via a node method is an issue. And if the library is not node-based, why would it be calling node components? I don't get it.

If the argument is that the node library, which is a less plugin, can't operate correctly using certain syntax, then why can't the plugin provide the needed syntax? Why change imports to support this unique scenario? But, this is a fair question:

is this actually a thing you've tried to do? working with nested packages containing LESS. If not I can understand why this might seem out of scope.

Yes, but not in a node context. Less itself has the power to enable a package-based design, in that imports can be optional based on configuration variables. There are no good examples of this, since most Less libraries now produce an "all or nothing" scenario (Bootstrap is probably the most notorious), and simply by including the library, you dump a massive pile of CSS into your output. But this is a design decision; it's not an inherit design result of the Less language. You could have hundreds of .less files and packages for a library, and for any given project, you could import maybe 20 of those based on vars, and of those 20, you could toggle whether or not there was any CSS output.

Sure, if you're fixated on those individual components being installed via npm, it gets a bit trickier. But it sounds like the reasoning for that is to actually switch on / off those components by the fact that they are added as a node dependency. I'm saying this can all be done without Node involved at all.

AND if you still want Node / npm involved, and that's fine, then make your library a single Less plugin, which provides that component architecture, and installs other dependencies as needed. I think the fixation is when, while trying to do something with @import and a node directory, things broke down. I don't think a custom @import needs to necessarily be involved in the scenario of loading a component.

It's possible I'm misunderstanding the problem. If that's the case, I want to get out of the way. I know that plenty of times a Less feature seems obvious to me, but if someone hasn't done that particular thing a bunch of times, they may be confused what the issue is.

So, I believe you that you've encountered a real problem; the question is how best to address it, and whether to address it in core, in a plugin, or in your library (or a combination of those). My understanding is that since 2.0, any system can implement a custom file manager. I don't see any reason why, like node, requesting a file couldn't also search the node_modules context. Plugins are extremely flexible. Functions can be added at will. File loading behavior can be altered. Less sheets can be injected. So I'm still confused how this can't be solved in a plugin, or why solving it your plugin (library) would be an issue.

@matthew-dean
Copy link
Member

Wait... are you saying this is for components for which you aren't creating the source .less? Or maybe you are, but are creating the equivalent of someone saying, "Hey, here's a dropdown library. Please insert this CSS into the head of your web page." Except in this case, it's a less file, and the dropdown library (UI component) is npm-installed, and you want to give an equal one-line reference to the .less? And you don't necessarily want to "copy" that less file, because you want to be able to update it via NPM?

So, more or less, you have distributed .less files in various node.js packages, and you want to find and gobble them all up in a less compile process? Or, rather than "find", you want to just reference each package folder or less subfolder.

If so, that makes more sense, but I think the part (if I remember correctly) about also importing that package's dependencies and being able to reference the nested less subfolder and also import it... that feels like it nudges out of scope again. Even in Node you can't require a module that isn't directly installed in the root node_modules folder for a project, can you?

If I'm understanding this better, then still, the nature of that workflow pulls it back to a plugin, with that plugin possibly scanning dependency folders or looking at a setting in package.json. It just seems fairly specific rather than a generalized feature.

@JedWatson
Copy link
Author

Thanks for the feedback @lukeapage and @matthew-dean, I'm really glad to be working together to find the solution.

re: having the plugin enabled by default, I think that would remove a big pain point because at least we could know that when less.js is used in a node environment (so, express middleware / webpack / gulp / grunt / command line / etc) above a particular version it would be fine. And if it's not in a node environment, node_modules isn't really going to come into play :)

I've seen the libraries (bootstrap, ionic, flexbox grid, etc) that are published as plugins, and honestly that scares me. I've got more than 30 packages in mind, and since the entrypoint is already taken by the js component, doubling that to publish bootstrap plugins for each would be maintenance hell. There's no need to wrap a set of LESS files with plugin infrastructure if we can simply include .less files directly from node_modules reliably.

The other scary thing is that the plugin infrastructure (even as light as it is) is duplicated for each package. I've just done a cursory review of the less-plugin-bootstrap code and found it suffers the same problem I've faced with react-select: it assumes that the bootstrap package will be installed in its own node_modules directory which is absolutely not something that is safe to assume.

I learned this in production with Keystone when my users started building react apps with it, and started using packages (e.g. react-select) that I was depending on in Keystone, and the path to that package changed in their environment. Lots of people started getting hard errors compiling the LESS for the Admin UI and freaked out. This is why I wrote that custom resolution logic that modifies variables after resolving the path. Now Keystone's LESS is bullet proof :)

Even though that approach can be made to work, I don't think it's the right solution, because it has some big scalability issues.

There are no good examples of this, since most Less libraries now produce an "all or nothing" scenario (Bootstrap is probably the most notorious), and simply by including the library, you dump a massive pile of CSS into your output

I think we're the first to try and develop a big UI library with modularity as a core design goal, without abandoning CSS stylesheets (others have gone the "css in js" route), and LESS is a great language to facilitate it.

I've used bootstrap for years, by the way, and always selectively decide which parts to include. It works fine when you're dealing with a monolithic library.

However we're trying to bring the unix (/ node) philosophy to UI kits, with lots of small pieces that do one thing really well and all work together.

While it's fine for a user to opt-in to bootstrap buttons and grid without forms (which means they need mixins and variables), the system breaks down if you want those things to decide which other parts they need.

Lodash is actually an interesting example of this, jdalton has done a really good job of breaking all the functionality into small pieces so you can get the whole library, or individual functions. lodash.camelcase and lodash.kebabcase both depend on lodash._createcompounder which contains the underlying functionality that's shared between the two.

Applying that concept to a UI library, you could have a button, and a date picker that uses the button and some common mixins, and a select field that uses the common mixins.

If you include the button, you just get that. If you include the date picker, you get the button and the mixins automatically. If you include the select field, you just get the mixins.

Then you could also include the mixins yourself, which would hoist them out of the date picker into your project. This is where I think node's module resolution system works, and is much more powerful than a flat structure of conditional includes (whether hard-coded or controlled with variables).

(Bootstrap has created their customizer to deal with this, which is helpful if you're only using Bootstrap components, but again doesn't scale, and doesn't work for non-bootstrap components).

My solution means that someone else can create a different date picker using the same mixins and button, and things still just work for someone who includes it in their project.

As it stands, it would be really difficult for me to publish a replacement to Bootstrap's built-in dropdown while reusing Bootstrap's base LESS (mixins, variables, etc) without explaining to the users that they need to include Bootstrap as well (and probably documenting which specific parts they need). It would be even more difficult to publish another component that included that replacement dropdown... you end up recursively documenting requirements the end user has to hand-craft (whether it's implemented with plugins, variables, or includes) and it becomes unmaintainable.

The best solution I can come up with is granular packages, that themselves have control over which other packages they depend on (and whether the contents of those packages is referenced, or output).

Unless I've missed something?

Sure, if you're fixated on those individual components being installed via npm, it gets a bit trickier. But it sounds like the reasoning for that is to actually switch on / off those components by the fact that they are added as a node dependency. I'm saying this can all be done without Node involved at all.

Sort of. Those components aren't enabled by being a node dependency; they're enabled by being included in the site's less file.

Hypothetical user's site LESS:

// My Awesome Website
body { background: pink; }

// Bootstrap grid ftw
@import "bower_components/bootstrap/less/grid";

// I use react-dete-select for forms
@import "node_modules/react-date-select/less/component";

// Make it match my theme
@react-date-select-bg: pink;

Now, since react-date-select also uses elemental-button and elemental-modal, its component.less would look something like:

@import "variables";
@import "dateselect";
@import (node) "elemental-modal/less/component";
@import (node) "elemental-button/less/component";
// make the other components match our theme
@elemental-button-bg: @react-date-select-bg;

Then, in their javascript, they'd use the date picker component that the react-date-select package exported. It doesn't really matter though that I'm working on a React library; it could be a jQuery component, or they could include the prebuilt version and just drop it into a <script> tag.

Or it could work with just a CSS library, no Javascript components at all.

You just need to be getting the packages from npm and build their LESS in a node environment for this to work (which at this stage, is something I can live with; if you don't want that, I bundle a prebuilt CSS file with every component anyway, you just have to override the defaults with more specific classes rather than being able to theme them with variables).

@JedWatson
Copy link
Author

you are creating the equivalent of someone saying, "Hey, here's a dropdown library. Please insert this CSS into the head of your web page." Except in this case, it's a less file, and the dropdown library (UI component) is npm-installed, and you want to give an equal one-line reference to the .less? And you don't necessarily want to "copy" that less file, because you want to be able to update it via NPM?

Yes! It's important that the CSS is versioned with the Javascript; they work together to make the component.

Also, I want to be able to do it recursively so that my date picker and my select field can (for example) use the same mixins (or button, or whatever) from another package.

So, more or less, you have distributed .less files in various node.js packages, and you want to find and gobble them all up in a less compile process?

Well, it's more like I want to say "Please import /react-date-picker/less/component in your LESS file" rather than "Please insert this CSS into the head of your web page".

There's no finding or gobbling, it's just a bunch of @import statements that make it work, and we use node's resolve logic to figure out where the files to import actually are.

To be clear, everything I am talking about is working today, and we're already prototyping Elemental like this. We've just hit this roadblock that we need resolve to find the file to actually import.

Even in Node you can't require a module that isn't directly installed in the root node_modules folder for a project, can you?

That's not something I want to do. I want it to walk up the tree of node_modules to find the file it's trying to import.

The reason I want this in LESS Core is that I can't do anything programmatically to include the existing plugin. Even if my package declares it as a dependency, less exists at a higher level, and require would fail because there's no way for it to know to look in my package's node_modules folder. Less may even be inside another package, e.g. gulp-less and then we're really stuffed.

@seven-phases-max
Copy link
Member

We've just hit this roadblock that we need resolve to find the file to actually import .

Btw. speaking of which... There's paths option (and it's supported by most of less tools), so technically you can just find path of the (for example) elemental-modal module within you script (one of those if I'm not mistaken), then assign elemental-modal's parent directory to less.paths and viola:
@import "elemental-modal/less/component"; works as-is.

@stale
Copy link

stale bot commented Nov 14, 2017

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
Copy link

stale bot commented Mar 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 Mar 24, 2018
@matthew-dean
Copy link
Member

Closing as implemented in 3.x.

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

No branches or pull requests

6 participants