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

Fix currentFileInfo and index properties on nodes #3602

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

bjpbakker
Copy link
Contributor

These fields are accessed by other tools such as parcel (for resolving local
asset URLs). As per 257efdd the behavior with
(at least) parcel for relative paths in sub-directries changed. Prior to that
commit (last release 3.12.2) assets were resolved relative to the less module
that contains the url(..). From release 3.13.0 parcel resolves assets relative
to the root less module, because no currentFileInfo is available.

This is caused by tree nodes setting their prototype to an instance of
Node. This leaves the self reference in Nodes constructor pointing to the
prototype, not the actual instance the data is set on. Replacing this with
properties defined on Nodes prototype fixes this.

These fields are accessed by other tools such as parcel (for resolving local
asset URLs). As per 257efdd the behavior with
(at least) parcel for relative paths in sub-directries changed. Prior to that
commit (last release 3.12.2) assets were resolved relative to the less module
that contains the `url(..)`. From release 3.13.0 parcel resolves assets relative
to the root less module, because no `currentFileInfo` is available.

This is caused by tree nodes setting their prototype to an instance of
`Node`. This leaves the `self` reference in `Node`s constructor pointing to the
prototype, not the actual instance the data is set on. Replacing this with
properties defined on `Node`s prototype fixes this.
@matthew-dean
Copy link
Member

Makes sense, have you tested with Less 4 to see if it has this issue?

@bjpbakker
Copy link
Contributor Author

Makes sense, have you tested with Less 4 to see if it has this issue?

Yes, I actually discovered this in 4.1.1, verified it's also on master, then traced it back to 3.13.0. This patch works for master, didn't test any older releases. But if you like to fix this in another 3.x release I'm happy to backport it.

@matthew-dean
Copy link
Member

Sounds good, thanks!

@matthew-dean matthew-dean merged commit b37922c into less:master Feb 8, 2021
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

Successfully merging this pull request may close these issues.

2 participants