[10:59:50] good morning/afternoon [11:00:00] Hey! [11:02:22] hi [11:03:30] just waiting for mikesherov now [11:03:39] seems that jeff is also not here [11:06:38] so jeff likely can't make it [11:07:37] hi [11:07:38] sorry [11:07:42] irc was acting weird [11:07:56] OK, agenda [11:08:28] nzakas: gibson0421 ariya ikarienator [11:08:32] ready? [11:08:41] * ariya is born ready :-) [11:08:45] 2.1 released! [11:08:55] Modules, RestElement, AssignmentPattern landed in ESTree [11:08:59] ready [11:09:05] Excited to see progress in both areas! [11:09:18] yay [11:09:22] anyone else have major announcements not needing discussion since last time? [11:10:01] Moving on... separation of concerns: [11:10:17] that's my proposal [11:10:33] should we keep requiring filing a ticket or should we just discuss on the PR [11:11:09] I think now that we have the PR as a reference in the commit, a ticket is not always required [11:11:20] some topics are delicate, it will be nice to keep the PR discussion only about the code review [11:11:22] often it is helpful to have some implementation as start of discussion [11:11:27] I tend to like requiring an issue [11:11:36] that way if the PR doesn't work out, you still have a touchpoint [11:11:41] OK, I'm fine with continuing requiring an issue [11:11:55] mikesherov: you can still point to your experimental branch in the issue [11:12:01] yes, this is true [11:12:06] I think trivial things, e.g. editorconfig, can go in directly [11:12:15] right [11:12:21] yeah, things that dont' really require discussion or design [11:12:24] also, when bug is found, reopening a PR feels a bit weird [11:12:29] ok, so we're not going to be completely hard nosed about it [11:12:38] but in general, tell them they should file issue as well. [11:12:54] I'm +1 on that [11:13:03] if we can extentd the jquery bot so that it automatically mentions that [11:13:10] e.g. if the commit message doesn't refer to any issue [11:13:49] yeah, I think we can extend *travis* to check for that in pull requests [11:13:57] and have it be part of the pass/fail check [11:14:26] welcome jsoverson [11:14:26] but travis can't post a comment on PR [11:14:37] hi mikesherov [11:14:38] I see. [11:14:42] related with https://github.com/jquery/esprima/issues/1066 [11:14:48] coverage is already checked with npm test [11:14:58] but then coveralls.io can post the actual numbers right there (inline) [11:15:00] right [11:15:06] ok, fair enough [11:15:25] lets move on. [11:15:32] I imagine this could be useful for other jQuery projects [11:15:36] yes [11:15:40] lots to do for jquerybot :-) [11:15:57] Shall we talk 2.2? [11:16:14] sure [11:16:28] easier when we look at the big picture: https://github.com/jquery/esprima/issues/1099 [11:16:30] heres link to main tracking issue: https://github.com/jquery/esprima/issues/1099 [11:16:33] jinx! [11:16:57] caridy1: has volunteered to port modules to master [11:17:03] he's had practice in espree ;-) [11:17:17] wants to port directly to master, and not to harmony [11:17:43] well he's had practice in harmony a long time ago, to begin with [11:17:53] yeah [11:18:03] so let's have him do it [11:18:08] will start working on that tomorrow :) [11:18:17] hooray! :-) [11:18:33] Generators should be 2.2 as well, right? [11:18:35] caridy1: make a note to claim it on https://github.com/jquery/esprima/issues/1000 [11:18:53] mikesherov: depends on FB, haven't heard from jeff [11:19:05] should we swap it with e.g. template literal? [11:19:13] template is shipping on FF and Chrome [11:19:17] I was going to say "both" [11:19:28] I can do template string [11:19:52] I just finished fixing all tokenization of template string: https://github.com/jquery/esprima/pull/1104 [11:19:57] with help from nzakas [11:20:04] w00t [11:20:15] so I think I'm fairly comfortable with porting to master trivially [11:21:09] I added a fairly complex test: raw`hello ${`nested ${`deeply` + {}} blah`}` [11:21:26] ariya: gibson0421 ikarienator interested in taking any? [11:21:37] mikesherov: FYI, template scanning in Shift is also pretty good [11:21:38] not just yet :\ [11:21:41] standalone tokenizer? [11:22:07] mikesherov: I plan to tackle internal refactoring [11:22:17] similar to https://github.com/jquery/esprima/issues/1111 [11:22:20] template literal alone is simple. Standalone tokenizer might be impossible. [11:22:30] it's amazing what you start to find using type inference tool [11:22:44] We can probably remember how many level of braces we have. [11:22:56] ikarienator: I did just that [11:23:24] So as far as 2.2 goes in terms of es6 features [11:23:30] modules, templates... ? [11:23:56] ikarienator: https://github.com/jquery/esprima/pull/1104 solves standalone tokenizer [11:24:13] Cool! [11:24:24] just keep a stack of { vs. ${ [11:24:33] can detect what } is then [11:24:52] ikarienator: care to take any es6 stuff? [11:24:54] Well, you don't have to. Just remember how many of them should be fine. [11:25:00] ariya: no es6 stuff for you for 2.2? [11:25:04] What's left :) ? [11:25:13] https://github.com/jquery/esprima/issues/1099 [11:25:18] ikarienator: lots of stuff [11:25:23] mikesherov: es5 is more fun [11:25:33] the only thing not in harmony is destructuring I believe? [11:25:36] ikarienator: you should tackle destructuring? [11:25:42] nzakas: that's the only thing espree hand to self implement, right? [11:25:48] OK I'll take that. [11:25:53] destructuring was partly copied from Harmony [11:26:00] and then we fixed a bunch of edge cases [11:26:24] you sure? Thought it was done by hand in espree [11:26:38] yep! [11:27:06] oh, it was missing defaults [11:27:10] that we did by hand [11:27:11] Hmm.. I'd like to see the impl of espree. [11:27:26] ikarienator: would be good to leverage that stuff [11:27:33] of course, espree looks more like harmony [11:27:36] it's open source, what's mine is yours :) [11:27:41] in terms of the node factories, et.c [11:28:02] ok, so ikarienator you'll do destructuring, including defaults? [11:28:10] Yes. [11:28:18] here is the big question though, ariya [11:28:46] modules will be ported directly to master by caridy, destructing with defaults will be done directly in master by ikarienator [11:28:56] are we ok with not having those back ported to harmony? [11:29:20] if we need them backported, I can handle that at a later date [11:29:30] jeffmo probably wants that :-) [11:29:33] more questions for jeff [11:30:01] OK [11:30:05] so I'll collab with him [11:30:10] for now, we'll go directly to master [11:30:15] and can backport if necessary [11:30:29] https://github.com/jquery/esprima/pull/1104 ikarienator or ariya review please, so I can land in harmoney and then port to master [11:30:38] probably easier to create harmony from what will be 2.2 [11:31:00] mikesherov: perhaps ikarienator can look at it? [11:31:08] ikarienator: you're more familar with template than I am [11:31:13] ok, that's fine [11:31:23] https://github.com/jquery/esprima/issues/1012 ariya can you fix the arrow edge cases for 2.2? [11:31:29] that stuff is in the wild :-) [11:31:30] OK I can review it. [11:31:53] What are the edge cases? [11:32:01] listed in the issue ikarienator [11:32:02] mikesherov: which part of 1012 specifically? [11:32:34] ariya 1010, ikarienator 1056 [11:32:41] Hmm. That probably can wait until we have 2-phase parsing [11:32:56] I can still give https://github.com/jquery/esprima/issues/1010 a try [11:33:04] yes, sounds good [11:33:15] and ikarienator https://github.com/jquery/esprima/issues/1056 is the simplification of arrow and group [11:33:38] I think this is done [11:34:06] the fix for rest has fixed it [11:34:23] oh, ok! [11:34:28] so close 1056? [11:34:38] ikarienator: PR 1093? [11:34:48] Yes. [11:34:51] Close it. [11:34:52] https://github.com/jquery/esprima/pull/1093 [11:35:02] ok [11:35:07] closing 1056 then [11:35:16] Yay! [11:35:50] ok, so just 1010 for arrow functions to be complete :-) [11:36:28] ok, updated https://github.com/jquery/esprima/issues/1099 [11:36:46] seems like: destructuring, modules, template strings, arrow edge case, and maybe generators [11:37:17] generators are easy, yields are hard [11:37:28] do we want to support both at the same time? [11:38:00] Besides, I think the tree definition for yield is not completed? [11:38:00] the tasks can be separated, but it doesn't make sense to have half the part [11:38:19] yes, let's push forward on stree for generators [11:38:37] estree* [11:38:48] that's why we'll wait on FB to help [11:39:16] I'll move es6 tickets into milestone [11:39:31] ariya: ikarienator feel free to move other stuff in to 2.2 [11:39:47] What's the time frame? [11:39:48] sounds good [11:39:58] ikarienator: yesterday :-) [11:40:02] Ah, good call. I can have template strings done hopefully tonigh :-) [11:40:28] Then I'll be moving things out of 2.2 instead :) [11:40:31] all I need is your approval on my harmony PR ikarienator and then I can move over [11:40:45] OK. [11:40:48] let's again shoot for 2 weeks, and move back if needed? [11:41:10] OK [11:41:21] That is, two weekends. [11:41:59] so meeting 2 weeks from now, let's try to be done [11:42:17] let's move on [11:42:18] mikesherov: for 2.2? that seems to be too rush [11:42:32] ariya: how about 3 weeks then? [11:42:39] 2.1 was 3.5 weeks [11:43:12] more like 4 weeks [11:43:33] let's shoot for 3 and miss rather than shoot for 4 :-) [11:43:37] sounds good [11:43:44] kk [11:43:46] moving on [11:43:49] commentAttachment [11:43:58] nzakas: ariya where are we at? [11:44:10] It's all in https://github.com/jquery/esprima/issues/1068 [11:44:28] ariya suggested an approach [11:44:36] I'm going to create a PoC with that [11:44:55] the biggest design question is: should we start exposing the delegate for everything? [11:45:06] everything is at least (node, token, comment) for now [11:45:09] see also https://github.com/jquery/esprima/issues/1068 [11:45:15] where the callback for a token is also requested [11:45:45] prior art: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Parser_API#Builder_objects [11:46:08] though the granularity of that builder object is mind blowing (= too details) [11:46:25] yeah, that seems like a tank when we need a squirt gun [11:46:45] builder objects is how recast works [11:46:48] we don't need that [11:46:58] how about just node, comment, and token? [11:47:06] the first two are already needed for custom comment attacher? [11:47:27] yeah...I'm not sure it's worth bundling token because I don't know what the use case is [11:47:34] it would be easy to add it later, though [11:47:37] I thought we'd be passing in a set of callbacks that come along for the ride [11:48:14] I'm missing the big picture here. [11:48:23] we're still talking callbacks [11:48:43] ariya is suggesting we should do the same think for comments, nodes, and tokens [11:48:53] the onComment should receive the comment object every time a comment is detected, and the onNode should be called every time `this.finish()` is called, essentially [11:48:59] should I create a ticket? Probably easier with code examples etc [11:48:59] right [11:49:00] I see [11:49:21] we need comment and node [11:49:25] yup [11:49:30] the question is should we also do token [11:49:39] I'm just not sure it's worth doing token without knowing what it would be used for [11:49:40] easy to expose it when needed later [11:49:49] yup [11:49:51] for now, let's just expose node and comment [11:49:54] ariya: ? [11:50:12] nzakas: there is https://github.com/jquery/esprima/issues/1019 for token request [11:50:25] mikesherov: sure, let me create a ticket [11:51:17] hmmm, seems only tangentially related [11:51:19] Oh, so consumer of esprima wants to save memory by not having to save all tokens to an array first [11:51:37] Yeah, I mean, I'm spiritually fine with revealing everything eventually [11:51:48] I think that's a more complex use case [11:51:53] lets start with comment and node so we can focus on comment attachment [11:52:03] and then tackle tokenization separately [11:52:09] OK? [11:52:32] https://github.com/jquery/esprima/issues/1113 [11:52:50] ok by me [11:53:17] lets move on, running out of time [11:53:38] feature flags, language detection..... [11:53:48] https://bitbucket.org/ariya/jsparser/ [11:54:03] this is the PoC of a second pass parser [11:54:07] Wait a second [11:54:14] ok, waiting :-) [11:54:35] We cannot move to having the callbacks until we have parser/tokenizer classes [11:54:57] Can we? [11:55:02] yup [11:55:06] ikarienator: we can, this is scaled back version [11:55:12] take a look at my PoC in the issue [11:55:21] Ah, that restriction was for generators... [11:55:24] ikarienator: hence why I tend to call it delegate (as in delegation pattern) [11:56:20] we can refactor a bit later, but for now, we have `this.finish()` being called on every node already [11:56:36] and `skipComment` is concrete comment creation [11:56:41] What about cover grammar? [11:58:14] not sure what that means... ariya ? [11:58:38] let's continue technical discussion in the actual issue [11:58:53] want to spend a few minutes on the language level / feature flags stuff [11:59:37] cover grammar is something that it is first parsed as one thing then at some point converted to another thing. [11:59:54] ikarienator: ah, that is a good point [11:59:54] ikarienator: we'll discuss that at https://github.com/jquery/esprima/issues/1113 [11:59:56] like group expression and arrow function paramters. [12:00:02] ikarienator: very good point [12:00:19] let's discuss that in ticket [12:00:22] and object/array literal and object/array binding patterns. [12:00:26] yeah [12:01:28] •mikesherov> feature flags, language detection..... [12:01:28] 2:53 PM <•mikesherov> https://bitbucket.org/ariya/jsparser/ [12:01:47] this is example beginnings of implementing language detection as a second pass [12:01:50] ^^^ a demo of the second pass [12:02:00] unfortunately, the performance will be bad (rather obvious) [12:02:00] yeah, second pass = slower [12:02:10] that's what ultimately led us to feature flags [12:02:15] it's worth noting that shift is doing 2-pass to become faster and more correct [12:02:22] carrying around less state in first pass [12:02:44] worth noting though nzakas, because I predicted you would and had same concern [12:02:48] Haven't finished that yet. [12:03:03] We will be releasing ES6 parser today. [12:03:07] woo [12:03:14] oh yeah, I'm sure there's a way to do two-pass that isn't slower [12:03:24] but slapping on a second pass to Esprima most certainly isn't the way to do it [12:03:26] also, consider https://github.com/shapesecurity/shift-parser-js/blob/es6/test/early-errors.js [12:03:32] nzakas: imagine if that second pass was exposed to you [12:03:44] none of the parsers (Esprima, Acorn, espree) correctly recognize all the early errors like Shift [12:03:55] yes, shift will be faster and more correct :-) [12:03:55] Then we will start to extract early error checks outside in TPP milestone [12:04:22] nzakas: consider the idea that having to use estraverse, etc, goes away [12:04:41] I'm not discounting the value [12:04:47] just that if the tradeoff is performance, I can't use it [12:04:59] right now, esprima does one pass [12:05:00] if there's no perf tradeoff then I'm a big +1 [12:05:02] I believe it will be faster. [12:05:14] esprima does one pass, then you do second pass with estraverse [12:05:21] the other alternative is to sprinkle the language checks + early errors inside the core parser [12:05:23] Offline algorithms can be only faster than online algorithms [12:05:39] mikesherov: yup, and you're adding a third pass with these early checks [12:05:49] we did a bunch of work to remove extra passes early on [12:05:52] nzakas: not if you come along for the ride! [12:06:13] it's quite a bit more complicated than that [12:06:25] that's why you're here :-) [12:06:27] In fact, in some cases offline algorithm can be faster than online algorithm in time complexity, not only constants. [12:06:40] estraverse lets us listen on ascending and descending routes through the tree [12:06:51] and we use that quite a bit [12:06:53] ah, so, "exit" [12:06:54] Don't use estravers. [12:06:57] yup [12:07:35] like, "do this after all arguments of a call expression" [12:07:36] You don't need to use estraverse to traverse your own tree you defined. [12:07:52] my bottom line is I really don't care how it gets done so long as I don't notice from a perf-perspective [12:07:52] ikarienator: the point is that if we wanted to expose our second pass [12:08:06] so a consumer could attach a callback to implement visitor pattern [12:08:09] We should be able to. [12:08:16] currently, just "visit" isn't good enough for eslint [12:08:21] right [12:08:27] It can also be something like this: https://github.com/estools/esvalid [12:08:47] I think we're focusing the discussion on the wrong problem here [12:08:54] All the early errors can be checked with the AST except for one. [12:08:58] eslint has a setup that works well with a given AST [12:09:06] all I want is for that AST generation to not be slower [12:09:31] nzakas: yes, but you also want it to yell about es6 if you say es5 [12:09:33] If you don't check the early errors, it will be faster. [12:09:39] yup [12:09:51] nzakas: why can't ES6 check be a rule in ESLint? [12:09:52] that is the second requirement we're trying to solve without flags [12:09:52] but that process happens in a black box as far as eslint is concerned [12:10:03] ariya: because it's a second pass and perf-killer [12:10:08] and the thought is that it can be done faster without flags and a second traversal [12:10:22] mikesherov: if that's the case, I'm all for it [12:10:32] and that we can avoid even a third traversal with a callback, but that's only if we have "exit" events [12:10:42] nzakas: I don't think it will be slow at all. [12:10:57] I'm speaking from experience, not hypotheticals [12:10:58] ok, sounds like we need some perf tests here. [12:10:59] nzakas: I would love to have the benchmark numbers. [12:11:08] nzakas: even it is just like other rules that already exist? [12:11:31] yup [12:11:39] perf is one aspect [12:11:47] other aspect is black box [12:11:49] the other aspect is why push that responsibility onto eslint? [12:11:52] yah [12:11:53] we pass in text [12:12:00] and a parser parses [12:12:04] and should tell us if it's wrong [12:12:17] early errors are grey area [12:12:17] it's really not eslint's responsibility to tell you that your code is syntactically incorrect [12:12:21] but language level is not [12:12:30] yeah [12:12:41] the separation of responsibilities I think is important [12:12:45] not only for sanity, but also for reuse [12:12:48] right, should be fully esprima's responsibility to say "valid es6 syntax" vs. "valid es5 syntax" [12:12:52] otherwise everyone has to implement their own validator [12:12:53] yes [12:13:14] so again, here's my position: I have something that works great right now [12:13:29] if Esprima can do something to make it greater, I'm all-in [12:13:32] How about we integrate early error tests in the parser but in opt-out second pass? [12:13:59] ikarienator: that is fine too, but the second pass we're discussing is about language level [12:14:16] that is the crux of the discussion [12:14:47] Oh.. Not sure why language level is related to second pass? [12:15:04] Sorry :( [12:15:06] ikarienator: https://bitbucket.org/ariya/jsparser/ [12:15:41] ariya has implemented this as a second pass, that just looks for valid es6 only stuff and then yells if it finds it [12:15:58] so, the options for es5 vs. es6 are: [12:16:06] 1. feature flags directly in 1 code base [12:16:20] 2. one package that wraps the implementations [12:16:34] 3. second pass verification of language level [12:16:47] ariya has implemented as 3 in that demo [12:16:53] that's what we're trying to discuss [12:17:29] I see the intention... [12:17:31] Sorry. [12:17:50] ikarienator: but 3 only works if the core, relaxed parser is much faster [12:17:53] *if* we move all current early error checks out of first pass and into second pass, then we may speed up the parser enough to afford the second pass for language level detection [12:18:08] yes, ikarienator, we are saying the same thing [12:18:13] well, we need to implement all early errors first anyway [12:18:30] nzakas: you've implemented all early errors in espree ? [12:18:31] What is language level detection? [12:18:38] it's probably less critical for e.g. jscs, but more important for e.g. eslint [12:18:50] it's less critical, but still really important to us [12:18:52] after all, what's the use of a linter if it does not complain when the code has errors [12:18:53] mikesherov: no [12:19:07] nzakas: do you plan on doing that? [12:19:20] hard to see how it'll impact perf [12:19:35] mikesherov: some existing checks are too strict for now [12:19:35] haven't really made up my mind about that yet [12:19:49] mikesherov: as ikarienator points out, 'var let' should be allowed but current Esprima breaks on it [12:19:50] I generally have fixed things as bugs have been filed in eslint [12:20:10] right [12:20:18] so, what's the path forward here? [12:20:31] let's implement all early errors first [12:20:44] without that, we're optimizing for something that we may need to walk back again [12:21:25] so, this is then unfair comparison to espree [12:21:25] because espree only cares about what is getting reported [12:21:27] yes, exactly [12:21:29] having all early errors also avoid future regression when we're refactoring etc [12:21:37] yes [12:21:55] yup, only so many hours in the day [12:22:00] :) [12:22:04] ikarienator: once you guys implement second pass [12:22:12] can you report on perf differences? [12:22:17] OK. [12:22:31] nzakas: can you give us a benchmark we can run and a budget that we can work with? [12:22:44] in general, we want to be fast [12:23:17] yeah, we have a benchmark we use, I'd just need to modify it to not run any rules to find out the parse time [12:23:34] nzakas: do know any huge es6 files we can benchmark on :)? [12:23:37] I suppose we should just run eslint with espree vs. eslint with esprima [12:24:00] ikarienator: transpile Angular 2.0 to es6 :-) [12:24:04] lol [12:24:08] ok, so goal should just be fast as espree with 2 passes? [12:24:21] on Angular 2.0? [12:24:31]