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

Less @imports and possibly other entities should use System.locate to find location of module. #11

Closed
justinbmeyer opened this issue Feb 22, 2016 · 3 comments

Comments

@justinbmeyer
Copy link
Contributor

stealjs/steal#565 adds a LOCATE() macro where files like less and css can use it to change a url to a path located by System.locate. For example:

@import LOCATE("package/styles.less")

might get converted to:

@import "../node_modules/package/styles.less"

The code for that extension starts here: https://github.com/stealjs/steal/pull/565/files#diff-abc90bebf14055b4190455e9af84a13eR1.

The problem with LESS, LOCATE, and @imports is that the @imported fill will be processed with LESS which doesn't allow the LOCATE hook to run.

The solution to this is to write a less plugin. I explained this better here: less/less.js#2792

The solution is to make a less plugin that gets added when we "render" some less into CSS here: https://github.com/stealjs/steal-less/blob/master/less.js#L43

Plugins are documented here: http://lesscss.org/usage/#plugins

Notice that plugins can be added during render like:

lessEngine.render(load.source, { plugins: [myPlugin] })
   .then(function(output) {
    },
    function(error) {
    });

There are two options how the plugin could work.

Completely take over path resolution.

My guess is that the plugin will look like the npm-file-manager here: https://github.com/less/less-plugin-npm-import/blob/master/lib/npm-file-manager.js#L6

In this case LOCATE wouldn't be needed.

Look for LOCATE ourselves

In this case, it's likely we can use the "visitor" approach and identify when LOCATE is found and rewrite the AST less uses. I think this plugin does this: https://github.com/less/less-plugin-inline-urls/tree/master/lib

Take over loadFile

Similar to the npm-file-manager, we might be able to pre-process the source before it gets to less. Possibly by overwriting loadFile or loadFileSync.

This might be the easiest way of getting it done.

@nlundquist
Copy link
Contributor

Unfortunately the visitor approach doesn't work without monkey patching the LESS ImportVisitor that runs while the tree is still being constructed. Plugin visitors unfortunately aren't run until the tree is complete. Considering that I'll be implementing this as a file manager.

@nlundquist
Copy link
Contributor

Resolved to use new 'locate://' & 'pkg://' path schemes to denote module identifiers to rewrite. These changes will also replace the existing LOCATE & ~ behaviour in steal itself.

@nlundquist
Copy link
Contributor

'locate://' scheme has been implemented. this issue is resolved. a new issue can be opened when we add a 'pkg://' scheme.

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

2 participants