GitLab Pair Programming - Frontend, 17 Jul 2020

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Pair Programming - Writing an eslint rule for global .off()

Description

In this pairing session we create an eslint rule to detect that `off(...)` and `$off(...)` are never called with less than 2 arguments.
https://gitlab.com/gitlab-org/frontend/eslint-plugin/-/merge_requests/19

A

Well, shouldn't we check if eslint actually has a rule for off on and off events already yeah, but that's.

B

Not as much fun as writing our own.

A

B

A

Here it says we should use upstream rules.

B

A

A

Oh, my god, I didn't wrote one.

A

Doesn't seem so.

A

B

It seems like a really helpful rule. You know.

A

um Yeah, maybe stu is too much as it's called um right, not biased, but specific yeah.

B

I don't know I I I think so, but it seems really helpful in my opinion, but yeah it's going to be a little bit specific.

B

So so there's like a really cool way to test eslint rules and if we go to like some of our tests here.

B

B

So you're, like here's all the valid ones, and then you can say here's some. What invalid ones would look like, and then you just you just hit it. You know you run these parameters against the rule and there's the cool helper. That runs all those.

A

Well, basically, so what we're gonna do is gonna check out this project. huh What do you think, I think that's a good idea. Let's bring a terminal on this screen uh yeah. You can definitely leave.

A

Bootstrap view again, yeah right, yep, we use yard. Yes, we use yarn. We install on stuff.

A

Then you do code dot, we open this. Oh there is a script.

B

Yeah, you just create a rule.

A

Oh wow, is it clear in package.json, yes,.

A

Why aren't we doing.

B

This more often yeah. If we create enough rules, then we could just auto-generate all of our code.

A

With enough concern, have you ever read this? Was this language called the perfect pure language that would run itself and delete every piece of code that wasn't pure and like imperfect and pass the test and keep going recursively until it can delete itself.

B

That's kind of like the dream, you know.

A

Okay, but this is a very important moment: how do you want to call your rule.

A

B

I don't know um no, no global off.

A

uh Yeah, even off, maybe something like that.

A

Blah blah blah okay, so you created all this stuff for us, we're done wow. We wanted a error or a warning error. huh Yeah we wanted to air.

B

A

Event off right, yeah so remove it. This is the for remove and listening right or uh that's where.

B

A

B

So both the the dollar off and the regular just so my my approach to it was if a function's called off and we pass a string as the first argument, we better pass. A second argument: that's that's my rule in my head. If a function is called dollar off or regular off.

B

You it, you need to pass two arguments.

B

And I'm kind of I'm and for these rules I don't know how you feel about this, like I'm fine, if we disable rules and uh like, but it's just yeah.

A

B

It makes it more explicit, doesn't it yeah like it should we should have lots of bells and whistles to go off when we disable a rule, but sometimes it's okay, uh yeah.

A

Okay, so as usual, when I don't know something I open, whatever is already there just to see what's happening right now,.

B

This one will be interesting: the the require required one.

A

Yeah it's so this is about props right, so I thought it would.

B

How about yeah, how about we find all of these seem like they're, really complicated bulls? um How about we find? uh How can we find a rule that exists on like that's much simpler uh in the wild yeah in the estate called.

A

A

A

I don't know kind of case, maybe.

B

Yeah, that sounds great.

A

B

A

No, maybe we go for uh like semicolon at the end.

B

huh What about funk names, because that's kind of what we're looking at too right, punk names there.

A

You go, it does not name a name.

A

Okay, create contents.

A

So they get the source code.

B

I'm bringing in this here, obviously we get the source code.

B

Do we have to do this review files too, like that would be really wonky. I hope not.

A

Well, I guess now are we using which which offer we're talking about the remove event, listener or.

B

The from jquery, the ones on jquery and um and the the end view as well, the off the dollar off and off yeah. um But, yes, lynch rules run on view files like without any help, because I notice, if you go back to our project, we have like view versions of some of these rules.

B

Is that just for the view template.

B

Oh jesus, um oh, it is review template, I think it says, detect non-externalized screens and view template attributes. I think this is just for view. Template stuff.

B

Look at the way they did their uh contacts. The fine template body visitor.

B

A

B

Did they find it's that util.

A

um I guess oh yeah, yes,.

B

A

Game process service.

A

I don't I don't know what is any of this okay, I I.

B

Have some context.

A

B

B

Oh contacts, parts or services template; okay, okay, okay,.

B

So go back, go.

A

B

Github go back to our github example: we're looking at okay, if you scroll down to like so we got the source code, we're defining all these functions. If you keep going down.

B

Yes, yes, you see what they return here. um This is um a visitor pattern of how so eslint works. With this visitor, padding, you kind of say hey when you hit this kind of node call back this function, um and so this is how they're, whenever you hit a function expression, I don't know what the dollar exit is, but or not the dollar but the colon exit.

B

But whenever you hit a function, expression, here's how we're going to handle it, um and then we can use ast explorer to really help out with what all of that. What are we actually looking for.

B

This guy yeah yep, so I think any time we're doing a call so like here we're doing four each that's what we're looking for is that thing which I guess is a call expression.

B

Identifier well yeah, but I was thinking if we look for all identifiers like we're going to get everything. um So I was thinking looking for the call expression we want to get.

A

The um so called expression is what you're calling a function right. Basically,.

B

And we want to make sure hey if you're calling a function called off.

A

Is there is there any yes and rule that come to your mind that um examine now you're calling functions.

B

That does call expression. We can actually use github search to do for that. If you want to look for call expressions and uh eslint yeah.

A

A

Prefer object, spread yeah the function call spacing.

A

I don't do it. I don't know what call spacing is, but it's the that you need to have a space between the arguments. I think that's the function right.

A

You see yeah, you.

B

Have a space it's doing.

A

B

Oh, I think it's also, with parentheses, the space yeah, the.

A

Space cool so yeah, that's what this will do it jesus? How much.

B

There's no are we out of oh, we are at a test. This is a test file. Oh okay, that's why I was what what.

A

uh But this is not a test file. A lot.

B

A

B

Oh, oh, here's one there's a lib up. There.

A

Where oh there, it is prefer promise reject errors.

A

B

So you see there we go, we have um so check that out call expression, yeah and check out there. Here's call expression takes a note or check hey. Is it a promise, reject call and the way they do? That is what's the callee? Is it a member expression? Is the identifier yeah.

B

So what we want to do here is.

A

I think this is enough for us.

B

Yeah, that's very very similar to what we want to do. Instead of promise, we don't really care who it's coming from, but we really care about the property being off.

A

So this is basically like this right and it takes a note, yep yep correct. Do you think, should we just go like step by step, I'm good with that yeah we can abstract later um and um I suppose we will need to have a test now right, yeah. That makes sense.

A

B

Oh, it already.

A

Generates that is nice.

B

I kind of like you know: I've been I've been noticing I've been a little envious of projects that have their unit tests right next to the subject, I kind of really like.

A

That pattern, yes.

B

I I agree with you: I used to do.

A

B

I used to not like the pattern, but now I think I really like it.

A

So actually we can do like this right, um yeah.

B

Yeah, it's fine yep, there's going to be something interesting here too. um If we go to.

B

So we'll go back to the ast explorer, there's two cases. We need to be concerned about.

B

Oh yeah, it didn't our.

A

B

Yeah, of course the rule doesn't doesn't die. Do you think we can just run this somehow? No, it's! I think it's complaining about line 26 like we have. We don't have valid javascript there.

B

Where, on you see it's, it's you've started a quote, but it didn't finish. The quote.

B

A

Yeah, okay, so this should have failed. They didn't uh no, but my question is: can we run only the tests so because this is our console log right? Oh.

B

It has everything in it. Oh got it yeah. um I see what you're saying.

A

Let me maybe I can try like um you, can just pass in.

B

Like a while, maybe or yeah.

A

That's that's what I'm trying. I should wait. No because I don't see a way to say it right, yeah, okay, let's do the old school way. Let.

B

Me, let me see if the rule tester thing is from um it's an island thing. Let me see if there's a force.

A

Thing you can pass there, I don't know this is using mocha much under it. So can we? I don't remember, motion.

A

B

J, if we have oh but that's testing, that's testing a. um I guess yeah. If we got the uh test name.

A

B

So go to the out. I guess that's what you can do. If you go to the output, we can get the test name from it. Yes, this one yep yeah. I guess we can do that. Let's try.

A

B

um So before we go down, I want to show one other part that we're going to run into um here. Yeah we can actually, um we can actually go to our. We can write a test case for this first, uh so on yeah here so there's there, when we have code off like that, but there's also going to be off when it's part of a property.

B

So if we do code and then um like event, dot off or something uh yeah and then with dollar signs, our or the other is the other state is those both of those states. But then, with dollar signs is what we're concerned about as well.

A

Yeah anything anything really.

B

B

A

Yeah one two, three four, I guess oh, but here I left alert. Let me just.

A

You can you can see that I'm tired apologies.

B

Oh you're, good, um I'm getting that I'm getting a headache. I think I need to get some more coffee. I think my body's gonna tell me something.

A

B

This this looks more like it right. This looks 100 like it to me.

A

Okay, so if we go back to this right, it's probably reject call. So this is basically what identified the function. We should have something similar to this right yep. So what? If? What do you think if I just take this all right, I love it. Can I do it outside of the context now, let's do in the context. Just I don't know.

B

Why they're moving?

B

A

Do you do it inside.

B

Yeah, I don't know why they do it inside.

A

Remember expression.

B

So we don't care about the object at all. It's just the property we care about.

A

But uh-huh but object nine name pro. So if we go to our ask explorer right- and if I do e sorry, this is this will be a typical thing: right, e dot off, for example, correct.

A

So what is this identify a member expression called expression, name of okay, so yeah you're right, you need! We need to just go through to your property name. Oh right,.

B

Yeah- and I don't know if we want to also check the property type um or I don't know if that's important to us, like that example, checked both the type that it was an identifier. I don't know if that's something we're interested in also doing um or just the name.

A

I think the name is could be enough right and we know so member expression. We don't even need right. I think we do want that.

A

B

Well, I mean, I don't know, I feel like.

B

The more the more conditions we add, the less of a chance of a false, positive, positive yeah, that's also true, um so I'm I'm kind of fine. If we add more. um Oh nice, oh you're, already engineering.

A

That rocks a little bit.

B

I'm a fan: well, maybe we should make it a set. So we're not you know. So we have oh one complexity time, I'm just kidding you're fine uh yeah. That sounds. That sounds great. uh That works for me and.

A

Then the other case.

B

Though, if you go back to our ast explorer, the other case is if we were not part of a member, and I'm just calling off by itself like this yeah yeah.

A

Well so, in that case we don't need the member expression, that's the thing right. We can remove this completely. Oh wait! If it's a function that is called off, that property.

B

Is a special part of member expression? I go back to the ast explorer.

B

So then it's just identify, then we want to see or the node is an identifier with the name off. That's why I think the type is important like if the type, if the node that we get so from.

A

There you go call expression, callee property, which is type type identifier right, yes, and- and here you go call.

B

Expression, calli type identifier name, so it's a little different.

A

So it's either this right, yeah or or no dot. uh Identifier.

B

A

B

B

A

Oh geez he's already 50.

A

Oh we're almost right.

A

Here and or this and the end node callee.

A

Dot name nope yep, but it's off list.

A

Includes this close, close paint.

B

I'm a fan huh that makes sense that that sounds good to me. um Can we do one more? Can we add one more case to our.

A

Guys tests yeah, but let me wire this.

B

Oh, that's a good idea.

A

So what happens if I run this.

A

True true true well clearly, so we need to add some false positive right.

B

Yes, I think so yeah good idea. um The other thing I want to do is if we have like three dots, if we're doing a long member expression, um wanna double check that that would do it as well like if I do yeah this dot event, dot off or something or yeah. That's good cool.

A

All through minus the this one sweet.

B

Cool so then, so that's our first.

A

Kiss, if we're.

B

A

One go ahead, so we're going to put an if and all our work is going to be here right, otherwise, yeah fine. How do they you just need not to.

B

Return right, basically yeah, so the only time I guess use context.report to throw something: that's happened. So, if you go to the check, reject call checks, the.

A

B

Ever reject and reports it if it can't be an error. Oh this.

A

Is exactly what exactly? What do we need.

B

That's great, I just want to say hey. Do we have two arguments like that's so easy right? So let's do a check off call. I guess we can call it.

A

Yeah check, of course, huh yeah. That sounds good to me.

A

Okay, no, we actually, we want yeah. We need to check if call expression argument. Dot length is less than two right.

B

A

I try all the rest.

B

Is less than two we blow up?

B

Can it ever be called just with one like?

B

No that's what we want to avoid right, I mean, can you actually ever call off with? No sorry I meant, can you call it with no arguments? No, that sounds like a bad thing right.

A

B

Okay, yeah, then, let's, let's do less than two.

A

Gd reject an error. I guess.

B

Yeah, I think our our message id does that make a difference. Do we have messages that we like need to? I don't.

A

Know and isn't this a default to reject an error thingy.

B

How did we reject, in our other, in our local rules,.

B

Now let me check for report. Oh we just we just load the message like that. We didn't do a message id or something and that's cool. I think so. Let's just do just an inline message.

A

B

A

Maybe we don't even need data right.

B

B

Oh gosh, like can you end up doing like some cross-site.

B

Like does it escape your parameters, you're, passing to your es length like? Could you could you cause a lent error that runs some code on your machine? That'd be scary, anyways. So it's not it's.

A

B

Failing did not specify any.

A

um Can I just console the vlog.

A

Is not defined?

A

Oh, what the hell! Sorry very good.

B

A

What is this? That's not what we were expecting. I guess right, node type, literal wait! So just we.

B

Are passing the wrong thing.

B

Is it just to double check.

A

B

Change it to less than five, and we should expect the other ones to.

B

Fail did these other ones fail.

A

Should have no error, but at once, okay yeah. So what are we doing wrong?.

A

So these have two arguments and that's correct right.

B

Maybe, um oh, maybe, though, we're not specifying our report correctly, like maybe it's reporting it as a warning or something. So if we do less than two.

A

B

Like is there something that we need to add in our description of this rule that actually says: hey here's an error. Well, but this oh.

A

B

Type, oh the types isn't here: it's the meta, the dogs, okay,.

A

We can remove category, I guess um I don't know it's a bit weird yeah. That is weird.

B

Can we can we? I know you gotta hop off um if you need to hop.

A

B

Go ahead for it, um can we compare our invalid ones? What so it's saying did not specify? Can we just compare art spec to one of the pre-existing ones make.

A

Sure to do this.

B

B

Invalid, oh, are we supposed to say here's, the specific is that what's going on we're supposed to say, hey, we expect this specific message that may be it.

A

Let's try it real, quick and then I hope off and I push this to nmr to a branch so.

A

No error is not defined yeah. I think it wants us to okay, so okay, const error, it will ask real quick.

A

B

A

B

What's going on.

B

I love it. This is my this is my favorite eslint wool. Oh.

A

A

Cool I'm a fan: okay called expression, not literal cool.

B

Held of the type of know that it got it right, cool yeah, that's great, hey again! This was this was wasn't a whole lot of effort uh and it's green nice. Yes, thanks so much nico, yeah and yeah. Please push too um and yeah for like an initial review. I can. I could do some cleanup stuff if you feel like, um and then we can pass it to another maintainer yeah.

A

I open off of my meetings and I push it to a branch. Okay,.

B

Thanks nico, hey and enjoy your time off, yeah.

A

Thank you all right. Okay, bye.