GitLab Pair Programming - Frontend, 3 Feb 2023

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: FE Pairing - 20230203 - Toasty actions and Yarn2 tampering tests

Description

In this session we explore some approaches to creating an action for a Toast popup, and discuss an MR that adds Yarn2 support to our untamper-my-lockfile script.

0:00 - Creating an action in a toast popup
17:32 - Yarn2 support for untamper-my-lockfile

https://gitlab.com/gitlab-org/frontend/untamper-my-lockfile/-/merge_requests/38

A

uh So share my screen describe the problem. Yeah.

B

A

You want to again you can share your screen yeah, so this is the issue I'm working on. um So when you add an um sorry when you promote a comment to timeline event, uh you get a toast message and we're going to add um view event to the toast message, which seems straightforward enough.

A

um Sorry, um so then an example of that is here is a comment. You press this button to add it to to the timeline um and we'll get this so the way the toast message works is the first problem or interesting thing.

A

um So to add the action um you you can run it on click, um so you can't make that action a link, but you can give it an on-click behavior in which you could replace the window uh like URLs, which feels a little bit clunky and I couldn't find any other examples of us. Well. I found one example of us using window.location replace in the code base, um so I, don't love this, and also the action is an anchor tag anyway.

A

So I feel like I should be able to add a link to that anchor tag. So this is one issue I have. um Maybe I want to speak to this for now.

B

Yeah I would oh I'm surprised you can't put out of hrefs on there does, but I know that the event shows up on a separate tab right of that same page.

A

B

So it's not that we just.

A

Created the URL, the routing so that it can be done. It's.

B

A

Been emerged yet.

B

Got it so if you do location replace this doesn't do a whole new page reload. This will actually view router will will take over oh.

A

Interesting um I was just going to do a whole new page I.

B

Mean maybe that's.

C

It depends like if your whole page is routed with you. You should probably use view router and navigate to this. um If the page is, you know still using single rails views, you probably want to um You. Probably we have a helper for that. You would do like go there. It's also ugly, that it's in a JavaScript but yeah.

D

A

It's now it's now using view router or it will be as soon as the Mr is merged, um but it yeah anyway. It's going to use the view. Router and that'll be fine, but.

B

A

I don't understand how this action still works, but that's a slightly different. Oh Jesus,.

B

Right, just just yeah, just just let it live, it's gonna.

B

I would I would then just write. Add a comment either on the Mr here that test that, once the view router stuff test, this Behavior with the V rather thing so does changing the URL will view around or just take over and do its thing and I wouldn't I I feel like there's a 50 50 chance. It does either one and there.

A

Might be that will work because I just I just did that that's cool.

B

My question was I, don't know if we need to do like a push, State versus a replace or like something like that. I don't know exactly, but.

A

This sort of clunky behavior is okay, I.

B

Think so, especially because yeah well I wonder if, on data.

B

Should we get the URL of the that's? My next question, yeah.

A

C

So I think with.

A

You, oh sorry, it's.

C

Probably better with view router, if you you know, can push the state or whatever. However, you do it um it's better than an URL, because you know of all the things that can go wrong with creating a URL here, and you know, um but I think this. Isn't this like how our file list view works as well. I mean we're properly using view router there.

C

So if you can use the view router API just do it, I mean the alternative would be that we teach the toast function, how to do view router stuff, uh like on the top level, so to have it a bit nicer, but yeah I, don't know.

A

That's a really good. Ideally, if we're going to change the toast information, then we'll just make it like it's already a anchor tag, we'll just give it the URL to go to.

B

Right uh view, router knows how to can handle anchored tags like you, have a special anchor thing. Oh okay and so I think that's might be what's going on with the finalist thing, but the file this one is a good example to bring up that's doing something similar.

C

But, to be honest, if you look in using I, think you wouldn't use an a ref and call whatever, because the uh because you would use the view router component and under the hood. This is nothing different from calling the view uh router API right, whatever you call the JavaScript API or there's like some full component. That does the same thing under the hood.

C

um The only thing that I might see would be worthwhile. If we have that use case more often or the toast, you know doing something with your router, which might be the use case. Once we have more um uh stuff that uses your router.

C

um It might also need to write a little help, but it is you know, toast with view router action, or you know, toast The View router that abstracts the little hack that you're doing here. You know, if so yeah that you know it works, because the downside of teaching the toasts how your router works is. We would need to bring view router as a dependency into gitlab UI and teach it all these kind of things right, which might be tricky if you need to update view routes on the future or replace it with I.

C

Don't know the next shiny, front-end, routing.

A

Thing that makes sense and like the under the other reason why I thought it'd be much better as a link is because, like the way it works is you know, like you get view event, and this looks like a link, so I should be able to like command click and open it in a new tab, but I can't if it's a on tick.

C

Yeah, you can, you could look at the event and cook look if a meta button is clicked.

A

Okay, yeah. Yes, that's.

C

A

Mean this is just like, obviously, you're right. The best.

C

Most accessible thing would be, if you actually use the if you, if, if you can make it an ahref, but it would have the downside that if it's a normal link and you click it view, router would not do view router. And so you wouldn't be able to switch over um in the single page application. And you would force a Reload right.

A

Yeah, okay, cool that makes sense, um I. Think that's some options that I can investigate. I want to show you the next problem, which is like when you get the data. uh It has um the ID of the new timeline event, but that ID is like the um I. Don't know how to describe.

A

So the graphql gives us back like the timeline event ID, which I don't know how to turn that into your a URL so like when we create the um sorry when we. So this is the service to promote the timeline event. To a note- and we say, like um you know, it gives the it gives disinformation.

A

uh Of what I would like to do is add, like a I, think what I would like to do is add, like a timeline event link and then I'd like it to build that link, because we have the ID and I guess we need to give some service in rails the ID in order to get the path.

B

Yeah, there's a there's a number of ways: I think we could do this. We can let the front end be the single source of truth of it and that's the front end build the URL or we can let the back end be the single source of Truth button. Let the backend Do It, um the fronted one would be potentially easier um because you can effectively just create a new URL from the current one.

B

This change the current path, so that we're not so that we can go to the timeline which I don't know if that's like up a level or whatever, but you could do a relative path.

B

um I I find using the the all caps URL. The native URL object is really really helpful. I like creating new URLs that are relative to other URLs.

A

So so that would look like so here when we create the um someone would create you know, and then we um so this should pause in the console. So we have. This is the ID we get.

B

Yeah so we'd have to we'd have to take out the graphql decorations of that ID. If we wanted to do it from here or.

A

We have to like take this string manually, cut out the number and then.

B

Except we have helpers to do that, um I believe uh I, believe we have helpers to do that uh and it's also maybe we have instead of on the type just an ID. Maybe we have an I ID and that's what we should be using um instead, because that would be just the 68 part, but I don't know if that exists, for these kind of things.

C

Is the timeline event already a thing in graphql, or is this also new in this much request? It.

A

D

Timeline event type: they have an ID events promoted from node node incident. It.

B

Doesn't have an IID. This is that it doesn't look like it does. No all right, um yeah, I, think we'll we'll need to um grab a there should be a function. That's like convert ROM graphqlid. If we do a project-wide search for like from g dot, star ID, we might find something.

B

um Get ID there we go, there's one get from ID from graphqli.

A

Oh yeah cool, oh great, okay,.

B

A

Makes sense well I'll. Do it in the front end, that's way more simple,.

C

The question is just like with the timeline event type and if they now get like whatever IDs or if they get like an URL or whatever the question would be if we should just expose it on the graphql type as well right, like just thinking about the API about our graphql API, is that you use is where someone wants the URL from this thing. I don't know yeah well, is the URL not existent at that point.

A

C

Feel like it would make sense. Yeah just like I mean as a first iteration. The front end thing is probably good enough and you know, but you know I don't know which back-end developer is helping uh with those features. You could ask him hey. Could we expose whatever field we need here and then it becomes even easier in the future.

A

Yeah yeah, that's useful I think I can go from there.

B

um Let me, let me uh share one more thing about URLs doing this from the front end. um If you go to the actual you, you have the site running, I, think.

B

um Yeah, you can go ahead and press play through this or whatever um so I did talk about using the native all caps URL object to easily create relative URLs, because if I'm in this View I want to take them to the timeline view and add an anchor tag where we jump into it.

B

um Ideally something like that.

B

Well, uh because I'm realizing summary.

A

Oh just for extra information, this branch is based on the branch that already has the um oh.

B

A

B

My question is: I, don't know if I added on our Zoom chat, I, don't know if doing a relative or going to timeline is always going to work nicely, because um what?

B

If here's, my here's, the situation you might want to figure out when I add a comment to the timeline, while that's loading, I, navigate to metrics or something, and so now, I'm on a different level of nesting in the URL than I was in summary right, so we'll just want when we build the URL rather than building it off of like window location href, we might want to find something stable to build it off of you know what I'm saying.

C

You would probably build it not in the on click event, but outside of the on click event, yeah.

B

Yes, yeah. That's a great idea. Yeah, like you.

C

You would you would build the URL at the time. uh Yeah, oh shoot, I also messed up in the chat like you would need to build it before right.

D

Yeah timeline and then you would do a.

C

Url Dot, is it just hash I think it's just hash right.

B

Yeah hash, but we could use window location, href I mean it might be. Even it might be interesting if it might, because maybe there's other way, there's other URLs, where you can create this from. If we had a stable incidence path that we got from the back end like maybe that's the most reliable thing for us to do, um yeah so rather but yeah, all of it to say, I shared also in the chat.

B

uh James I shared um a link to this toast function and I'm surprised that href isn't a thing because I if I saw that I would immediately start going to middle click. The thing uh the view event because I imagine someone might be trying to go through a list of them and then it's like. Oh, let me middle click. This I don't want to lose my current context, but this wouldn't let me uh middle click on it. So.

C

Also, maybe a gitlab UI issue.

B

Yeah exactly so I think I I can see us wanting to add an ahref here.

C

um I also don't understand why this is a splice and not an unshift because notes, dot supplies, zero, zero is so confusing you just.

D

Added something.

C

And and the whole render title thing like, why is it so complicated like.

B

I don't know I didn't put together the splice00. That's interesting yeah. This is.

C

A that's just adding at the beginning of the array.

B

That's interesting, yeah yeah, so this that would be that's not obviously not something that needs to be done right away, but that could make your other MRI easier. Just things to think about.

A

Cool thanks cool sharing.

B

Do you want to move on.

A

B

Yeah, let's do it do you want me to share my screen? I appear: do you want to share.

C

It sure um so my wife is going to come soon and I have to drop off eventually, but I just have one main point in that merge request. um Maybe we can jump into that one quickly.

B

Let me um share my screen and close some things out and yeah. Let's go for it here. We go sharing screen, oh wait, this is git live UI. Get out of here, get live uh all right. Yes, what's the point.

C

So the point is about the ul's: if you go to changes, maybe you should I should.

B

Let me just quick, quick, oh you're, good I'm, hearing changes.

C

um Okay, so um the past package name. So this is the thing. The past package name is a little interesting um because apparently, after the initial part of the name right like the beginning here, loaded or gitlab untemper, there is an ad and after that ad comes in URI and the thing with the npn. What you're doing that you're? Basically converting that Soul number into that Alias kind of thing is interesting, but I think it's. Maybe the the wrong approach in um the whole thing is looking at the thing a little bit backwards.

C

I think it would probably make more sense in the yarn versus two strategy to be like hey, let's pause this thing, you know this name and the resolution thing and the only thing that we support is workspace. Where we're just like: okay, we don't care and npm where we are like okay, we need to double check that everything looks all right and nothing. No one has done something bad and we I think.

C

We also need to need to support like the patch URI scheme yeah, because this is what they do to typescript and a few other packages to.

A

C

Npm compatible and potentially in the future- and this is also something that untemper currently doesn't support at all, but you could also have the git and the https URI schemes. If you, you know, get your packages from elsewhere, yeah, so.

B

The intent here is for some reason uh the NPA was parsing this. If I had something to look like this, it tried to fight. It tried to treat 4.1.7 as the package name. Did that make sense. uh So that's why um that's why I changed it to this, so that NPA would Would Not freak out when it would have found this part. Let.

C

Me just one second yeah.

B

You're good man, I knew I was afraid of offending IP, but I could tell he was really upset. With my last comment. He just stormed out of the room I'm joking.

B

I'm sorry I didn't mean to offend you so much IP I'm. Joking, no, all good.

C

So I I think there's a little confusion here. So imagine.

C

B

C

uh It's my daughter here.

C

C

Okay, good um so I think there's just a little confusion here, because npm has also a feature to do aliases. So, for example, if you do, um if you want to install view 2 and view 3, you can write, you can write view to add npm colon View at right.

B

C

Looks the same as this resolution thing here right.

B

C

Actually isn't so if you, if you would, if you would think that, let's imagine that yarn versus two would have chosen not npm but yarn versus two as a as a name space, you know the whole. The resolution would, for example, look like a yarn uh for 70 21 I'm. Just writing it in the chat, um while an npm, alias that you actually give to NPA would look like something like this.

D

C

Let me just uh ah view two is the best example view two yeah uh View, at which version of view do we have uh I, don't know 2.7, 10 or whatever right so yeah yeah, I I wrote the wrong. I wrote the wrong thing. Sorry again: well,.

B

Well, I think the Alias, the Alias they're, pre-existing alias Behavior, makes sense.

D

B

Thoughts was in interpreting. This was that yarn two is just I, don't know for some reason, assuming that an empty Alias meant just use the same name but I do think you're making a good point of like conceptually yarn resolutions are just seemingly a little bit different than these than what this expects of its package name at URI and I. Think that makes sense so.

C

Like it's yeah, so I, actually you might have done the right thing here. So basically you changed the npm thing where they also have done a shortcut into the uh into the full qualified URL kind of thing, maybe to put it differently, I! Think if you have, if you have a package Json in mind right, a package Jason has, on the left hand, side low, Dash and, on the right hand, side for 1721 right and the at npm colon here that we have is basically just like the normal colon in the package.

C

Yeah yeah yeah, you get what I'm saying like it's just like conceptually it's a little bit interesting and I. Think um because we have the patch Shenanigans going on anyhow, we probably need to rework the yarn versus 2 strategy a little bit um to also give better errors if we encounter an unknown protocol because imagine yawn supports downloading something from FTP right.

A

C

Should throw an error that untemper my log file doesn't know how to do FTP stuff, yet right right.

B

So I'm kind of part of me is thinking. Yeah um part of me is thinking. I need to look for the first at word, character, colon and whatever is before that is going to be the package name.

C

Yes and that's what I've done here, you can see the regex and the regex is using named capture groups. Yes, so the only confusing thing is the thing might start with an edge for the scoped names right. Then you have the at before the scheme. You know I called it scheme like you, know: hdp yep, SSH, git, all schemes and then there's a colon.

C

uh The first colon and after that we just call it hierarchy in the URI RFC um and higher hierarchy would be like the full domain URL or would be like you know, yeah.

B

Yeah, so in this case then it just sounds like we wouldn't even need to really use NPA I, like that I think it's.

C

Great, let's do it? Yes, you! You would need to do this and basically, if something the encounters, a scheme that we don't know, you know we throw an arrow um because on Timber might look for this is by the way, a limitation at Untamed by my look fat at the moment also has in the normal resolution, like we don't allow. If someone uses a package directly from git or from an HTTP URL right, it should be a feature request and the patch thing here, I think it's very interesting.

C

I looked at the log file of the web IDE thing, maybe you should just in the first iteration uh uh whitelist allow list the the the thing, the stuff that um how do you call it the stuff that Jan does internally, they I.

B

C

Only patch resolve FS events and typescript. So it's like a limited list that you probably can hard code somehow, okay,.

B

That's a good point: I am planning on adding as a e to E. Yes, input, the web ID log file. Yes, the current one, I'm gonna, add that as part of the ete's thing, but this makes a lot of sense and I I get this and it's nice that the only thing we're trying to do is just parse the package name. So I I think that's a good point and that's something that's something to do. That would be. That makes sense.

B

C

Other and it's one thing I did want.

B

To highlight yeah, oh sorry, let me highlight one thing: um does it make sense, I, don't know if you how how much you read into it, but I found it to be a lot easier. If, when I'm parsing, the yarn lock file, yes to then send to the validator, let's.

C

B

Get rid of all the workspace entries because otherwise.

C

B

Because otherwise, you're gonna need a whole bunch of like. Oh, are we a workspace entry? We gotta allow it. Then it's like I was going down that path. I was like I need to just get rid of all these entries because there's no way to Tamper them.

C

Yes, all good, then then this is this is like a like a valid solution uh for the workspace entries, but the the other thing in the validation. You probably need to write like a switch case statement for the other protocols right and everything. That's not part of the npm workspace uh npm.

B

C

Basically- and the patch probably is probably invalid, as in the first iteration right, yeah,.

B

Yeah yeah, so that's I'm I need to get that feedback by putting the whole I knew. I was doing this iteration and then I know. Okay, I need to try this against the whole thing so I was I was excited to see that e to e thing, I'm gonna, try that out and then I know. That's gonna get already failed by the way, with the.

C

Patch thing it all failed like this.

A

C

Why I looked at yeah yeah, so one thing that you also should do because I don't think you do it, but it's something that I encountered with version, one that you basically uh need to do something like. Oh where's, the Alias thing I always forget how to do it. It's poorly documented, um npm, install.

C

Yeah, let me just I mean you have the example uh in the npm docs that you basically oh geez, do something like I'm pasting it in a chat that you do something like this in a maybe in in um so that you do yeah. Do something like the second line um and just see how uh and add and enter and test with a with a log file that just has like a.

B

Point thing: yeah: on a yarn tail doing the that's a good point.

C

Yeah, just how it looks, because it could be that it just we need to rethink a little bit. How we do the thing yeah has.

B

Ete this yarn too, with feelings. Yes, yeah.

C

And just copy the npm install command because nobody knows how to write them. That's a good.

B

C

Encountered the whole thing in real life when I started I think in gitlab UI. We have it now because we installed view two and view three or we install multiple versions of um bootstrap view. This.

B

um Yeah this one's my favorite, like the only reason to do aliasing, is if you need more jQuery and you gotta.

C

Have no that's not true. The only reason to use aliasing is- and this is also something that you also- we also maybe should test in a future iteration or even in this iteration is I, don't know if yawn 2 allows you to use this Alias to you know, use pulse, Slaughters, Jake, very Fork, and then you know at Paul Slaughter jQuery, and then you know you being able to force overwrite all the uh Jake queries that are in your dependency tree with your fork right. This.

B

Is, oh yeah, that's obviously that's the that's the long-term goal. Yes,.

C

And there's another another tiny thing that I said: um Jan versus one lock files has an explicit array of optional dependencies, while um yarn 2 just has like a meta attack and says: hey, this dependency is optional. So if you just just go into your young log file of the web IDE you will and search for optional, you will see it or I actually linked. It did I link it. No.

B

Yeah, let me open them real fast.

C

Yeah, if you just search for optional.

D

Yeah, you can see it I see, it depend.

C

ah There are peer dependencies, that's interesting, because we should also look what to do with them if they, if you can tamper with them, that's a good follow-up, but you can also see that they're like optional dependencies right yeah,.

B

Interesting, okay, uh so how? What do you think is a good way to cover these so I'm interested in wanting to get this working for yarn V2 like get this integrated to the product as soon as possible?

B

So what to what scope? Are we interested in covering all of the yarn, V2 cases and then.

C

I think the first good iteration is basic.

C

I um I think the first good iteration is, if you, if, if the thing passes for the web IDE, it's probably good enough for first iteration and we couldn't call it the beta feature and, to be honest, like I, have all kinds of test cases inside the test files um where I dealt with. You know peer dependencies and bundle dependencies and all these kind of things.

C

um Maybe it makes more sense to move them to package jsons again and being able to generate like how the log files would look. You know what I mean like um to basically have the same end-to-end coverage for each case in foreign.

B

That would be interesting um and that's a good point though too I think I can I can copy a lot of the current ones and create a yarn two version of it and see yeah what happens there? That's an interesting point: I.

C

Mean once you have, the first version are satisfied, I'm, also more than happy to try to break it by just adding more test cases right, yeah,.

B

That's yep yep yep, that's what I was thinking. Let me ask you one more thing and it sounds like you. Gotta, uh maybe hop up. Yes, the one thing I was going to ask was I noticed a part that looked at pack your package Json. So it looks like the lock file but also looks at the package. Json.

C

B

Well- okay, maybe not so that's what I would like to get some clarification on am I interpreting this correctly. um Let's just check uh uh here.

C

Oh no, this is a little bit confusing, so the package is called package.json and it just retrieves the metadata from the registry. So the thing that does this is basically calling npm and getting back all the data from npm who's.

B

Alive, yes, okay got it for some reason: I thought it was reading the package for some reason: I thought I was reading package Json all right, I'm glad it's not um because because of the workspaces, that would be a challenge, because we would have to read all of the workspaces package Json if we were going to like map these.

C

We once did it like we once scanned package, Json uh and I tried, like I, threw like arrows if a dependency was missing and these kind of things and then I just realized. This is this is dumb, because if you just do a yawn install, it will complain about missing dependencies. Anyhow right, like so I mean with untemper my lockify, the only thing that we're really trying to cover- and this is also the reason why I didn't look into yantu, because I didn't use it enough.

C

It's like you just try to cover the cases that Jan versus two doesn't catch itself right.

B

C

So like, if you, if you um yeah.

C

So the only like, if you have something that is like you know, the yarn immutable and if you have a temper block spider that is like a young beautiful that would uh raise an error. It's okay, um then.

B

uh They don't have to recover. That makes a lot of sense, yes, and so on that. On that note, um this, this I wrestled with this. A little bit is that yarn teals doesn't do the same Integrity check as yarn one, because yarn2 saves everything in zip files, so it actually saves the checksum of the zip files, which is really kind of frustrating. But then, when I tried like okay, can I tamper with this in any way, and you just can't like yarn doing yarn immutable is just going to if.

C

The check is basically so they basically the only way to temper. Is you change the resolution and the hash? At the same time,.

B

Which would work hatching resolution, so that would be why it should be fine, so I yeah.

C

B

C

The the only the only thing that you that you check in the end with the yarn 2 version is that the resolution is as expected: yep yeah. Basically have you tried to tamper with a log file like have you just changed.

B

Yeah you can't change, you can do the typo squatting is a.

A

Thing: okay, that's.

B

That's um yeah so cool thanks for taking a look at this I really appreciate the quick feedback on it and this. This is a good idea, so um yeah I'll I'll.

C

Go through I'm just happy to see more contributions because you're getting going to get like much requests in reverse.

B

Oh yeah um yeah I'm, uh I'm really eager to get the because right now, I have Mrs closed on the web. Id projects I'm really eager to get those open because I feel bad. That contributors aren't able to make contributions. So cool appreciate everything.

C

Cool, thank you. Have a nice weekend. Take.

B