Fix nth of type

[kilobyte2007]

The current implementation of nth of type is not working correctly when there are more similar tags deeper down. Example:

<ul>
<li><a>Link 1</a></li>
<li>
    <a>Submenu</a>
    <ul><li><a>Sublink</a></li></ul>
</li>
<li><a>Link 2</a></li>
</ul>

In this case, the correct :nth-of-type of the <li> of the Link 2, would be 2 because only siblings are considered (as per https://developer.mozilla.org/en-US/docs/Web/CSS/:nth-of-type), but because in the previous implementation querySelectorAll(tag) was used, it was also considering the deeper nested elements so it would return :nth-of-type(3) instead of :nth-of-type(2).

[fczbkk]

@kilobyte2007 I'm not sure if I understand correctly, but I don't think this is going to work. Can you please add a test case for this? Something like this:

<div id="root">
  <div>
    <p id="grandChild"></p>
  </div>
  <p id="directChild"></p>
</div>

const root = document.querySelector('#root');
const directChild = root.querySelector('#directChild');
const result = getCssSelector(directChild, {
  selectors: ['nthoftype'],
  root,
});

I don't think result will be what you expect it to be.

[kilobyte2007]

The case you provided will indeed fallback to nth-child because div:nth-of-type(1) > p:nth-of-type(1) is valid for both p tags so that's not the case I was looking to fix, as this is already correct.

I have actually made a mistake in the original comment, should've been "it would return :nth-of-type(4) instead of :nth-of-type(3)".

I have added a test, but it won't show up the incorrect selector for the current implementation as it falls back to nth-of-child cause the nth-of-type selector generated is invalid (hence the fallback) but you will see it generates a better selector in the my version.

[kilobyte2007]

Hey @fczbkk, sorry to bump this. Any updates here?

[fczbkk]

Hey @kilobyte2007, sorry about the delay. I went through your issue and I finally understood why it did not make sense to me. Your update will work fine with the current version. But I am thinking about a tweak to the algorithm that will potentially create shorter selectors by first considering general child selectors and only then try the direct descendant selector (that's what current algorithm uses). In that case, your change would be counter-productive.

I'll try to implement the new algorithm sometime this week. If it won't work, I'll merge your change.

[kilobyte2007]

Great, thanks!

[fczbkk]

Hey @kilobyte2007, I just published v3.0.0. It tries to prefer child selectors over descendant selectors, which means that in many cases, the selectors will be shorter and simpler. Please, give it a go.

[kilobyte2007]

Hey, @fczbkk great news! Will give it a go later this week. Thanks a lot.
But I still think this PR needs to be merged because the nth-selector still won't work correctly in some situations. Example:

var div = document.createElement('ul')
div.innerHTML = '<li class="a1"></li><li class="a2"><ul><li class="b1"></li></ul></li><li class="a3"></li>'
div.querySelectorAll('li').forEach((el, i) => console.log(el, i))

In this case, the .a3 element will have a :nth-of-type(4) selector with the current implementation while it should be :nth-of-type(3) because the nested .b1 should be ignored. Or am I missing something?

[fczbkk]

@kilobyte2007 I finally had some time to play with this. You were right. This problem should be fixed in v3.4.4.

[kilobyte2007]

Thanks a lot I'll try to use the v3.4.4 version.