Skip to content
This repository has been archived by the owner on Oct 2, 2020. It is now read-only.

Add missing arguments to loadFile and loadFileSync #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add missing arguments to loadFile and loadFileSync #25

wants to merge 2 commits into from

Conversation

parshap
Copy link

@parshap parshap commented Aug 23, 2017

I am using less.render in synchronous mode (using the syncImport less option) to preprocess css module files using css-modules-require-hook. However, when using this plugin, less synchronous mode stops working -- the callback passed to less.render is never called.

After some digging, I found that this is because less's import-manager.js calls loadFile with a callback argument (see here) and file-manager.js calls loadFileSync with an encoding argument (see here). The FileManager implemented in this module does not forward these arguments along, causing the calls to fail.

This PR fixes this module when using less.render in synchronous mode by adding the missing arguments.

Less's import-manager.js calls loadFile with a "callback" argument [1]
and file-manager.js calls loadFileSync with an "encoding" argument [2],
without which operating in synchronous mode (using the syncImport less
option) is broken.

[1] https://github.com/less/less.js/blob/4a0026ebf6d4c783365d3701ac796c18dcc9d30f/lib/less/import-manager.js#L119
[2] https://github.com/less/less.js/blob/4a0026ebf6d4c783365d3701ac796c18dcc9d30f/lib/less-node/file-manager.js#L32
When running in sync mode (syncImport option), loadFile should call the
callback instead of returning a promise. This change implements logic
similar to [1].

[1] https://github.com/less/less.js/blob/49cbe520f6b38b4b8797dcba491501c0c461c7f5/lib/less-node/file-manager.js#L31
@parshap parshap changed the title Add missing arguments to loadFile and laodFileSync Add missing arguments to loadFile and loadFileSync Aug 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant