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

Refactor LESS to separate JS "environments" from the core library #1732

Closed
matthew-dean opened this issue Dec 15, 2013 · 17 comments
Closed

Refactor LESS to separate JS "environments" from the core library #1732

matthew-dean opened this issue Dec 15, 2013 · 17 comments

Comments

@matthew-dean
Copy link
Member

As noted in several issues (If I or others have time, please reference those issues so we can close them later when this is addressed), LESS assumes a Node / Rhino / Browser environment based on specific tests, and enables / disables features or changes the behavior of those features based on those switches.

This has several pitfalls:

  • These are not the only JavaScript environments available in which to run LESS, so the detection gives false positives for environments, and false negatives for feature support.
  • The detected environment may or may not have the capability of the feature. A browser is assumed to never have capabilities which it might have in a local webview scenario.
  • Some Node environments have been known to trigger a false positive on another environment, meaning updating / maintaining detection methods.
  • The existence of the switches causes code bloat

The solution:

  • Allow environment plugins to be included in LESS builds, so that developers can build LESS in new and unknown JavaScript environments (such as Adobe AIR, TideSDK, TideKit, Chrome Native Apps, etc).
  • Keep the LESS core library clean (remove all Node/Rhino/Browser references), so that parsing of LESS / CSS output is its only job. File / directory access, or any external I/O is the job of the environment plugin
  • Allow contributors to submit environment JS plugins to the LESS repo, so that LESS supports the needs of more developers ongoing.

Just as an example how an environment plugin would function differently: Right now, LESS might want to get a file. If LESS decides you are a browser, it makes an HTTP request. If LESS decides you are Node, it calls the Node File API.

Instead, LESS would be agnostic about the method. LESS may call a function getFile, seeking a return object. The environment plugin can either say "not supported", return an error, or return file contents. The LESS library would be responsible for handling the behavior if a feature is not supported, or the data could not be retrieved. In the case of Node, the Node environment plugin would call the Node File API, and return the file contents in callback function referenced in the original LESS call.

Related Issues:
Issues #1392 #1339

@lukeapage
Copy link
Member

aww I thought this was a pull request :(

Yep, this is my next big project unless someone starts it before me - I have mentioned this on the rhino bug too.

@matthew-dean
Copy link
Member Author

Haha, no sorry @lukeapage, I just realized that I had never made a proper issue for it. I have some developers interested in approaching it, but they'd be new to contributing to Less. Is there a single-page guide for which branch to pull down, how to test, and submitting guidelines? Or maybe we should get all of us on a Skype call?

@shovon
Copy link
Contributor

shovon commented Jan 12, 2014

Allow contributors to submit environment JS plugins to the LESS repo, so that LESS supports the needs of more developers ongoing.

Or maybe have less.js be its own repo, and all the other environment support are separate repos. I'm not a fan of one big repo maintaining different environment, even if developers are able to rip the core out, and add it to their own implementation.

@matthew-dean
Copy link
Member Author

Maybe. I could see the node environment at minimum packaged with Less to give the most common use case and to document via example how to build an environment plugin. Maybe rhino and browser, although cases could be made for splitting those out.

@matthew-dean
Copy link
Member Author

@shovon @lukeapage We could do something like what Ace does, which is to have the core lib in one place, and then include a symlink to it in other repos, OR (probably the easier-to-maintain scenario:) we have a separate repo with environment plugins (maybe moving it to less-plugins?), so there's one place for people to make pull requests just for plugins. Thoughts?

@lukeapage
Copy link
Member

Lets think about that after the work is done. Once things are seperated it
will be easy to seprate into different repos.

@lukeapage
Copy link
Member

I've started work on this on the 2.0.0 branch. The first step is extracting out the "environment" into 3 files all following the same api and also document that api.

@lukeapage lukeapage self-assigned this Feb 22, 2014
@matthew-dean
Copy link
Member Author

Yay!

@matthew-dean
Copy link
Member Author

I think what will be convenient is that "node" environments won't have to "test" for node. If you have Node, you use the node plugin/build.

@censys-git
Copy link

"Instead, LESS would be agnostic about the method. LESS may call a function getFile, seeking a return object."

Exactly my use-case. Trying to run this in a software-as-a-service environment where access to files (reading and writing) are performed using the vendor's APIs for loadFile, submitFile, and so on. My two cents if looking for use-cases... The file operations should be fully separated from parsing operations. For example, in the environment I am in, I cannot access any files I want to combine/parse with paths but instead with the internal ID of where it is stored in the database, and I cannot write out files with path names (I must submit the file using the object instance originally returned from a "loadFile" or "createFile" API call), so the plugin should expect back standard file object properties such as "content", "size", etc. but not necessarily "path" or "url", and expect them in standard/agnostic types (e.g. text, number) and not platform specific objects or types.

@lukeapage
Copy link
Member

See the 2.0 pull request for my work so far. I've been busy and had to
scale back my contributions, though I hope to be back to finish that work
soon..

@matthew-dean
Copy link
Member Author

@lukeapage I poked around. The separation is looking good. If I'm looking in the right place, I think loadFile can be simplified where it's not forced to include a recursive call and pass back in parameters, or those parameters could be simplified. So, basically the environment could just respond to the request for a file and pass the contents back (or a response code if error).

Otherwise, I like that functions / features are override-able, so that an environment could be "browser" (or function mostly like a browser), yet support data URI conversions.

Look forward to this!

@lukeapage
Copy link
Member

Do you mean make loadFile synchronous? We would have to allow async. Is there an environment where you can't even call the callback straight away.

@censys-git
Copy link

No, the issue is with how less.js goes about gathering up the files to process/parse and then how it saves them. I may not be using the appropriate language/wording but hopefully you can see my problem. I essentially want to do my own file collection and then utilize the less.js for parsing/combining, then use my own file APIs for saving back to the system.

The environment I am in does not store files in a traditional file system or file structure. They are stored directly in a database. Files must be read and saved using the environments APIs. My request was to separate out how you gather up the files to process and how you save/create them back to the system from the actual processing/parsing routines For example, you have your built-in file collection routines in a separate module that can be overridden with a custom one:

FileParser = (function(){

return {

readFile(…): function(){…},

saveFile(…): function(){…},

…

}

})();

If this is set then this would then call my FileParser and I would mimic any required methods if necessary. If it is not set, then it uses the built-in file reader/saving routines. You would just need to define the methods expected to be overridden and the parameters/return objects/values. The key would be to pass in for parameters or return simple javaScript objects/arrays, strings, etc and not things like a DOM element or standard FileSystem object, etc.:

[

{

"filename": "xxxx",

"contents": "xxx",

…

},

{

},

]

Just for some background for what it's worth, the environment I am in (SaaS), there is a set of controlled APIs for accessing the files. Even though in the UI may display them like a typical hierarchical file structure, in reality they are just a bunch of IDs in a database with a folder ID and parent folder ID and contents of the file stored as Base64 strings. There are APIs such as loadFile, createFile, saveFile. They use an environment specific file object that has some standard get/set methods like get/setFileName, get/setFileContents, get/setFileType, etc. I must use these to handle anything file related so hoping to somehow leverage the less.js where necessary and override where necessary.

I am not sure what you mean by load synchronous and callbacks. It is all server-side but uses javaScript the same way as any other script and Less.js is loaded in this context and fully accessible just as it is client-side with the exception that you cannot use AJAX and do not have access to objects/methods like DOM, document, window, etc. Not sure if that helps or makes it more convoluted.

Best.

-Steve

From: Luke Page [mailto:[email protected]]
Sent: Sunday, September 07, 2014 11:15 AM
To: less/less.js
Cc: sl1331
Subject: Re: [less.js] Refactor LESS to separate JS "environments" from the core library (#1732)

Do you mean make loadFile synchronous? We would have to allow async. Is there an environment where you can't even call the callback straight away.


Reply to this email directly or view it on GitHub #1732 (comment) . https://github.com/notifications/beacon/5711310__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyNTcyMjA3NiwiZGF0YSI6eyJpZCI6MjIwOTQwNTF9fQ==--d14e1ada951a2e910583b8c9e34051e04b698359.gif

@matthew-dean
Copy link
Member Author

@sl1331 has a good description. You should be able to pass in an object or library for file management that implements specific functions (such as returning file meta data e.g. modified date), return file data, return things like base64 converted data, and, of course, writing files. If that plugin is added, less uses it. And since anything like Node.js file operations will be a "plugin", Less MUST use at least one plugin for core functionality to even work.

I wonder if it might also be smart to make plugins non-destructive (I'm thinking like event handlers). So, you can add two file plugins that can separately return matches. (In the case of files, if both file handlers returns a file, the first one would be used.) I can come up with use cases, but my experience is that if you make everything "override" destructively, that architecture eventually implodes. Files need some special casing, but making a non-destructive plugin architecture (like less.plugin.add(FileHandler) rather than less.plugin.fileHandler = FileHandler) begins to really click when you think about custom functions, where different libraries may add multiple plugins of the same "type".

Apologies if this is the current approach already. I keep hoping to have some time to look at 2.0 but haven't found it yet.

@lukeapage
Copy link
Member

@sl1331 it sounds like you are confusing saving the output and loading an import. lessc is the file that saves the output and that won't be basic functionality of less.render - which will be the main way of "rendering" a less string to a css string.

I have made the environment replaceable, but that isn't the same thing as the plugins. So I envision that for basic less conversion if you wanted to run it within .net for instance you would do less.createFromEnvironment(dotNetEnvironmentAPI) and then you would have a less.render which used that environment for everything you described.
The file handlers will be handled as plugins that I think will be addable per less.render call and which yes, you will be able to add multiple file handlers. The only kind of issue with that is that each new file handler / plugin has to either use the environment api already defined or be environment specific (but I think thats kind of inevitable - I wouldn't want to enforce every environment had database access functions in it so that someone could write a database plugin - someone will write a database plugin that is node specific or browser specific or possibly even a plugin that can work in either environment).
I think that's a good compromise.

Anyway, whats in this issue is done - the environment is completely seperate from less now (in v2), though I do have more refactoring to be done.

@matthew-dean
Copy link
Member Author

👍

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

No branches or pull requests

4 participants