[12:01:21] gibson042, DaveMethvin, m_gol, markelog meeting [12:01:22] https://docs.google.com/document/d/1BadwSO6Sn91RBfr05s77r81ncEi0oMBJ5mZS-k215vg/edit# [12:01:26] hey-hey [12:01:34] hi [12:01:37] I've got a work meeting I'm missing right now, so this will be quick. [12:01:58] first up, https://github.com/jquery/jquery/pull/2627 [12:02:12] I am in favor [12:02:47] what methods are we talking about? [12:02:51] only jQuery.type? [12:03:06] I was about to ask :) [12:03:20] def type, right? [12:04:30] as long as we're consistent across browsers, I'm also in favor of the other ones [12:04:38] as Dave suggest, it seems we need to find all methods that are work with type sniffing [12:04:52] it seems there is at least three of them [12:05:14] I see isPlainObject, isEmptyObject, and isNumber specifically [12:05:43] so four [12:06:14] i wonder how shims are implemented [12:06:15] right, but according to m_gol's tests, it looks like we're already consistent. Do we need changes for anything but type? [12:06:38] if shims are pretty close, we should be fine [12:07:17] I'm not sure we need to worry about shims. They should be mimicking native behavior as close as they can. If they don't match native behavior, that's their problem to fix. [12:07:46] what if it is not possible to fix? [12:07:55] anyhow [12:08:08] then we can rethink, but it may be moot [12:08:09] i think we should worry about it only if problem present itself :-) [12:08:11] maybe they're fine [12:08:19] right [12:08:23] but it would good move to check in it [12:08:32] fair enough [12:08:47] plus we can add tests for those methods [12:08:52] so that would seem to be the next step [12:08:55] like m_gol suggest [12:09:10] but i wouldn't add babel dep only to test that shim ) [12:09:29] since it is big [12:09:38] sorry, missed the beginning [12:09:52] no, I'm thinking a one-time test just to check if workarounds are needed. If babel behaves the same way as native, we can just test native. [12:09:57] talking about https://github.com/jquery/jquery/pull/2627 [12:10:07] yep [12:10:48] seems as an easy change, if there is only four methods we should be checking there [12:11:11] present, sorry [12:11:35] if anyone thinks of more, please add them to a comment [12:11:48] m_gol: talking about https://github.com/jquery/jquery/pull/2627 [12:11:56] yeah for Symbol I'm fine with having those 4 handle it [12:12:01] markelog: babel is already added [12:12:07] for iteration testing [12:12:09] but no $(Symbol) please!! :) [12:12:11] but only on Node side [12:12:15] oh yea, we check iteration [12:12:23] which doesn't work with jsdom >=6 btw [12:12:25] DaveMethvin: agreed, that's invalid input [12:12:37] because it requires Symbol & I'm deleting global.Symbol to replace it with a shim [12:12:43] so I need to find another way to test it [12:12:52] it might require a separate test module that's transpiled [12:13:04] and if that's done, we might as well do testing of transpiled symbols [12:13:29] well, let's see if we need to test babel first [12:13:35] sorry, power cut [12:13:44] basically, this test won't work with jsdom >=6 https://github.com/jquery/jquery/blob/master/test/node_smoke_tests/iterable_with_symbol_polyfill.js [12:14:03] and changing the approach requires some work [12:14:15] we may not need any extra node smoke tests for this [12:14:31] wow, did i miss something? :-) [12:14:35] I'm saying that this existing test prevents us from updating to jsdom >=6 [12:14:44] so this needs to get changed anyway [12:14:52] oh [12:15:06] probably to testing transpiled output in real browsers and not Node+jsdom [12:15:26] I see [12:15:26] and when that's done, we might as well test transpiled Symbol, the additional effort will be tiny [12:16:00] well, I also want to test native symbol if it's available, not just transpiled es5 [12:16:31] but I see what you're saying [12:16:43] testing native will be easy [12:16:49] and I'd even prefer real browsers to node+jsdom [12:16:51] it doesn't even require eval tricks [12:17:07] cool, I think we're on the same page [12:17:26] like https://github.com/jquery/jquery/blob/99e8ff1baa7ae341e94bb89c3e84570c7c3ad9ea/test/unit/core.js#L1594-L1612 [12:17:49] yeah, this "delete global.Symbol" test was a hack, really ;) [12:17:56] since it was simple to do this way [12:18:08] but it worked only as long as jsdom didn't depend on Symbols so heavily [12:18:09] makes sense [12:18:37] So, https://github.com/jquery/jquery/pull/2623 getting proper window for getStyles [12:19:12] this is weird to me [12:19:13] would like some time to checking this out [12:19:14] more discussion in https://github.com/jquery/jquery/issues/2622 [12:19:32] since i proposed original fix for it [12:19:40] defaultView should have gotten us the right window? [12:19:52] anyway it does seem kind of strange [12:20:01] markelog_: ok, we can wait [12:20:19] I guess we need more investigation anyway [12:20:43] maybe ask help form mozilla ppl [12:21:02] since iframe styles was always flacky in ff [12:21:10] flaky [12:21:22] miketaylr can probably get us to a good resource [12:21:36] yeah, or boris :-) [12:21:57] timmywil: i can take https://github.com/jquery/jquery/issues/2193 off your hands if you want, i should have time this week to clean up my open prs as well [12:22:00] https://github.com/jquery/sizzle/pull/355 Lastly, I'd just like to get more eyes on this Sizzle PR. It's a nice speed-up, but it's changing something that's been there for a long time, so tell me if you think of an edge case this might break. Otherwise, I'll land today. [12:22:14] DaveMethvin: sure! that'd be great [12:22:38] Anything else? [12:22:52] how this pr came to light? [12:22:53] markelog_, m_gol ? [12:23:12] gibson042 found it [12:23:16] gibson042: don't forget about https://github.com/jquery/sizzle/issues/356 [12:23:29] markelog_: I believe https://github.com/jquery/sizzle/issues/337 [12:23:31] was this because of recent discussion? [12:23:49] oh, cool [12:23:51] okay [12:24:12] I don't remember if it was https://github.com/jquery/sizzle/issues/337 , I just had some occasion to be looking at that block and made a mental connection [12:24:15] I'm not sure what preempted it, but gibson042 opened this issue (https://github.com/jquery/sizzle/issues/354) and I got curious [12:24:36] nothing else from me this week; just waiting for a Sizzle release :) [12:24:36] because the browsers specifically optimize selectors starting with id tests, but not attribute [12:24:52] and for BrowserStack to make Safari 7.0 be Safari 7.0, not 7.1 [12:24:58] also, i didn't have time to start implementing ua-testing [12:25:08] will hoping to get something done tomorrow [12:25:32] that's cool, shows that there is always things to improve :-) [12:26:10] is there a perf suite for that pull [12:26:10] ? [12:26:28] I don't have a link. I just used the Sizzle speed tests. [12:26:41] oh, okay then [12:26:44] it's an area we can improve, but something is better than nothing [12:27:02] gibson042: that is the quote :-) [12:27:16] it's worth noting that our qsa path is the fastest of all the selector engines overall [12:27:26] not all selectors, but overall [12:27:51] so we're the best! [12:28:13] it is always worth noting stuff like that ) [12:28:19] lol [12:28:27] ok, thanks everybody. Need to run to a meeting [12:28:34] see ya! [12:28:36] ping me in -dev if you need me [12:28:38] thanks