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

Duplicated nested mixin rule when @importing multiple times #884

Closed
suisho opened this issue Aug 4, 2012 · 23 comments
Closed

Duplicated nested mixin rule when @importing multiple times #884

suisho opened this issue Aug 4, 2012 · 23 comments

Comments

@suisho
Copy link

suisho commented Aug 4, 2012

This issue is like #49.
When import same file multiple times, I got result that has duplicated nested mixin rule (only nested mixin)
For example

_mixin.less

.mixin() {
  color:yellow;
  .mixin-inner{
    color:red;
  }
}

some-block.less

@import "_mixin";

.some_block{
  .mixin;
  color:red;
}

hoge-block.less

@import "_mixin";
@import "some-block.less";

.hoge_block{
  .mixin;
  color:blue;
}

and result
some-block.less

.some_block {
  color: yellow;
  color: red;
}
.some_block .mixin-inner {
  color: red;
}

hoge-block.less

.some_block {
  color: yellow;
  color: red;
}
.some_block .mixin-inner {
  color: red;
}
.some_block .mixin-inner {
  color: red;
}
.hoge_block {
  color: yellow;
  color: blue;
}
.hoge_block .mixin-inner {
  color: red;
}
.hoge_block .mixin-inner {
  color: red;
}

but expected result

.some_block {
  color: yellow;
  color: red;
}
.some_block .mixin-inner {
  color: red;
}
.hoge_block {
  color: yellow;
  color: blue;
}
.hoge_block .mixin-inner {
  color: red;
}
@lukeapage
Copy link
Member

so use import-once or don't import twice?

@suisho
Copy link
Author

suisho commented Aug 6, 2012

I tried use import-once. But this issue cannot resolved.
And I can't care "don't import twice" when maintenance many less files.

@lukeapage
Copy link
Member

then why did import-once not work ?

@Heilemann
Copy link

A more fundamental question is, what's the situation where you'd want to import something twice? It has never ever happened to me or anyone on our team. Why would you ever want to have the same thing defined twice? That doesn't make any sense, and causes major headaches when trying to debug. It was causing so many issues for us, because of the sheer number of .less files we're dealing with that we simply forked LESS and fixed it on our version so that @import IS @import-once (which, by the way, didn't actually work on trunk).

@suisho
Copy link
Author

suisho commented Aug 7, 2012

The discussion is different from my thinking.

I think this problem is failed filter duplicate rule
that committed by cb78933 and resolve issue #49.
And I expect resolve this problem by fixed this function.

If this result is feature, I come to terms this issue.

@lukeapage
Copy link
Member

Can I close this and leave discussion to #212 ?

@Heilemann
Copy link

Yes.

@suisho
Copy link
Author

suisho commented Nov 8, 2012

@agatronic Issue of #212 is closed but this bug is not fixed in 1.3.1.

@lukeapage
Copy link
Member

pull request #1010 has been pulled into 1.4.0

@suisho
Copy link
Author

suisho commented Nov 8, 2012

@agatronic hmm... I can't think that pull request not resolve this.
Why that clear this bug?

@lukeapage
Copy link
Member

because the file will only be imported once and not twice, so the rules will not be duplicated...

why does it not clear your bug? What behaviour exactly is the problem or that isn't solved?

@suisho
Copy link
Author

suisho commented Nov 9, 2012

@agatronic
When I execute above description's example, output has duplicated rule.
I expected following result,

hoge-block.less

.some_block {
  color: red;
}
.some_block color:yellow .mixin-inner {
  color: red;
}
.hoge_block {
  color: blue;
}
.hoge_block color:yellow .mixin-inner {
  color: red;
}

I think that commit of cb78933 is not apply in nested rule (not ralete import once) and this issue isn't fixed.

@dmcass
Copy link
Contributor

dmcass commented Nov 9, 2012

First, you're missing a semi-colon in the above example which is causing the weird selector output. After fixing that, your issue becomes more clear. Checked out 1.4.0 and tried it myself.

hoge-block.css

.some_block {
  color: yellow;
  color: red;
}
.some_block .mixin-inner {
  color: red;
}
.some_block .mixin-inner {
  color: red;
}
.hoge_block {
  color: yellow;
  color: blue;
}
.hoge_block .mixin-inner {
  color: red;
}
.hoge_block .mixin-inner {
  color: red;
}

Essentially it looks like nested imports need to be addressed with the import once functionality.

File A imports File B and File C
File B imports File C

File C still gets imported twice.

@cweekly
Copy link

cweekly commented Nov 9, 2012

FYI/PSA: Twitter-bootstrap 2.x users run into this when they @import bootstrap.less and responsive.less into a single .less file. (They're designed to be compiled separately, so each imports variables.less and mixins.less).
When they're compiled together, responsive.less's (re)import of variables.less and mixins.less becomes redundant, causing undesired side effects like what's described above. So it's an example of duplicate @import in the "real world", albeit relating to naive attempts to customize bootstrap without understanding LESS, vs a legitimate need to re-import. /$.02

@suisho
Copy link
Author

suisho commented Nov 10, 2012

Thank you for advice and pont mistake. I update issue

@lukeapage
Copy link
Member

ok, I see there is a bug in import once, thanks @dmcass. I have re-labelled this as a bug.

@lukeapage lukeapage reopened this Nov 10, 2012
@Heilemann
Copy link

Which again begs the question why there are two import methods, and why the
default import behaves in a way that makes no sense what so ever.

On Nov 10, 2012, at 6:42, Luke Page notifications@github.com wrote:

ok, I see there is a bug in import once, thanks
@dmcasshttps://github.com/dmcass.
I have re-labelled this as a bug.


Reply to this email directly or view it on
GitHubhttps://github.com//issues/884#issuecomment-10253207.

@lukeapage
Copy link
Member

No, in the 1.4.0 branch and future release, the default behaviour has been changed to import-once

@Heilemann
Copy link

Fabulous!

Michael Heilemann

On Nov 10, 2012, at 8:10, Luke Page notifications@github.com wrote:

No, in the 1.4.0 branch and future release, the default behaviour has been
changed to import-once


Reply to this email directly or view it on
GitHubhttps://github.com//issues/884#issuecomment-10253677.

@kamranayub
Copy link

Has the duplicate issue/nested import issue (noted by @dmcass) been fixed as well with @import-once? I am just running into that now and was wondering if it was a LESS issue or the plugin I am using's issue. In order for the plugin (Web Workbench) to provide intellisense/code-completion, you have to import the files you reference. Obviously, this bug becomes rampant then, because the files you import also import the files they need to reference, causing duplicates.

Will the bug only be fixed in 1.4.x or the next 1.3.2 release as well?

@lukeapage
Copy link
Member

not yet, its part of the major blockers for 1.3.2 (everything labelled paths)

@lukeapage
Copy link
Member

I have committed a fix for import-once so that it uses the full path to determine duplicates, not whatever path is used in @import.. which may be relative.

@kamranayub
Copy link

@agatronic, thank you! Gotta say, this is the second issue I've brought up that you guys have fixed ASAP and I'm super thankful. Kudos and keep up the excellent work!

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

6 participants