#1661 Test vanillajs-2

[antonmak1]

1661 I made the code here just as an example, maybe it will help someone (You don’t need to upload it. I’ll delete it later after vanillajs new)

[antonmak1]

I think that matches can also be replaced with something faster, but I'm not sure about that.

[leeoniya]

I think that matches can also be replaced with something faster, but I'm not sure about that.

sure, e.target.matches('#clear') can be e.target.id === 'clear'. same for .className prop. you might need to do .className.indexOf() of .className.includes() is there are multiple classes.

[antonmak1]

I think that matches can also be replaced with something faster, but I'm not sure about that.

sure, e.target.matches('#clear') can be e.target.id === 'clear'. same for .className prop. you might need to do .className.indexOf() of .className.includes() is there are multiple classes.

Yes, I think it will be faster, but I haven’t tested it, but after filter and forEach I understand that it is sometimes better to write specific functionality yourself than to use such functionality (in terms of speed)

[krausest]

Cool - thanks! Screenshot for the currently relevant benchmarks (was unintentionally chrome 126): For those benchmarks it's just preventDefault vs stopPropagation, isn't it?

[trueadm]

@krausest I added comments to this without realising it was merged. Apply those changes and you should see a boost in perf :)

[antonmak1]

@krausest "For those benchmarks it's just preventDefault vs stopPropagation, isn't it?"

I just didn’t understand why preventDefault should be done for the button tag when this is usually done for links. stopPropagation just assigns the cancelBubble property to stop bubbling.

[antonmak1]

@krausest There are still many improvements that can be made, described above by @leeoniya and @trueadm. The community will need to test them and upload them as a pull request.

[antonmak1]

@krausest I don’t know, maybe @hman61 or @ts-thomas will tell about more what is better to do for vanilla-js. @localvoid says that "ivi and vanillajs is that when table is cleared, it removes rows by replacing DOM node with a new one instead of removing rows with textContent="""

[trueadm]

I'd also argue that https://github.com/krausest/js-framework-benchmark/blob/master/frameworks/keyed/ivi/src/main.ts#L109-L111 falls in a grey area here where no other major framework is doing this. Replacing the tbody with an empty one is O(1) in Chrome, whereas textContent = '' is O(n) looking at the Chrome code. There are lots of these whacky optimizations though, so it's up to you if the vanilla JS implementation should also do this.

[antonmak1]

Vanillajs should be an example for other frameworks, which is correct both in terms of speed and logic of operation. Everyone needs to think about this!

[antonmak1]

@krausest @trueadm "There are lots of these whacky optimizations though, so it's up to you if the vanilla JS implementation should also do this." - says correctly. You can even put stopPropagation on selectRow so that it goes faster without bubbling to tr and it will also speed up, but this is not correct, as I understand it.

[antonmak1]

I still don’t understand whether it’s normal to put addEventListener on #main? Shouldn't it be on window or at least on document?

[trueadm]

@antonmak1 It doesn't make any real difference in reality if the event listener is on the document or #main.

[antonmak1]

@trueadm If we take the bubbling process, then this will be several more parentNode's (if without stopPropagation). What I mean is that in most popular frameworks an event listener is attached to document or window. Maybe this practice could be used as an example for future frameworks? But, yes, you are right. Specifically, in this task it doesn’t really matter where the event listener will be added.

[krausest]

Please see #1661 for updates.

[antonmak1]

I think that matches can also be replaced with something faster, but I'm not sure about that.

sure, e.target.matches('#clear') can be e.target.id === 'clear'. same for .className prop. you might need to do .className.indexOf() of .className.includes() is there are multiple classes.

@krausest instead of matches you can try this too

[antonmak1]

@krausest no need to change tbody to a new one - this is garbage!