Browser hacking: Updating the Temporal implementation in LibJS

welcome back so today i want to work on the serenity us javascript engine lib.js but instead of building something completely new we're going to improve something existing and specifically the temporal implementation because they have been as you can see here a whole bunch of specification changes i think these got merged yesterday yes and so basically what i want to do is go through all of them and see if and if so how we need to update our implementation accordingly so let's jump right in the first one is going to be this one as you can see two of them i authored myself so i should be familiar with those um and these are generally just issues that came up uh either well we implemented it or others like v8 um are implementing it as well and then we all report these small issues we find and then i think once a month they meet and um agree on uh making these spec changes so the way this looks um we get like a source code change basically here so in this case they added a step this is not very friendly um for humans because like it starts with one and one and one so they turned us into a proper specification page which is this one and then turns it into all these steps so what i'm going to do is just find the algorithm which is this one basically and then look it up in the spec and then here you can see the human readable version basically so let's have a look where that is in the js um so let's just look that up the nice part about having specification comments is that these exactly match the specifications so whenever i find something in the spec i can find it in the code as well which is really neat as you see we only have um two steps and they now have a third step so we're basically missing this part and now require internal slot as you can see down here is really just a type variant so instead of allowing any this value we want a specific one in this case a calendar as you can see from this slot so initialize timer parallel calendar simply means it's a calendar object so what we do is basically just replacing it like this then we get a calendar object and if it's not a calendar object it will throw the try will abort this function and return and then instead of calling tostring we simply need to wrap it into a value and that should be it now let's see if that also works so um i will use lagom this time for most of the development which is a lib.js compiled on linux serenity is just an alias i have points to the meta serenity sh script so the way to run the tests on your host is basically just say run lagom as the target and then test.js it's really really convenient so as you can see we already have a failure here because apparently we specifically added a test to allow any kind of object which is now changed so we'll need to update yes as well yes works with any disk value so this is basically no longer the case it now specifically not works with any dust value other than a calendar so i think what we'll do is just add another section we usually call it describe errors like that and then we pass a function to that and inside that function we're going to define all our test cases um so i'm just going to look for an existing test which is going to be easier just say not an object of type and then let's see if we got any temporal ones yes down here just going to copy that like that so this value must be a temporal calendar object and then we simply say this so this is a bit tricky because it's using a getter and not a function so we use a reflection basically to access that property with a different disc value in this case a string um that is wrong i nested it like this and then we're gonna it's gonna call the getter internally with this value that is a string and then it should say not an object of type calendar let's try that again and as you can see so these are just um skipped tests so they output a warning but it's um not a failure so all of these passed and that's basically it and then what i do is usually just copy the first couple of characters here to put that in the commit message and user usually just call it um something like this is a normative change in the temporal spec and then see this commit and then let's see this is um [Music] temporal calendar prototype id so only allow calendar this value in this function and that should be all let's commit that failed okay now it worked because i still had to install prettier as we edited a javascript file and use automatic formatting there as well um but not clang format obviously all right let's uh look at the next one so this is done next up is this one remove duration handle fractions okay i'm not sure if we have that actually um oh yeah okay so at the um i'm not so the thing is we're not always implementing the exact spec so when we encounter an issue sometimes what we do is just fix it up front and then file an issue and then um in the likely event that the fix gets accepted as is we don't need to change it separately um so that's basically a trade-off remake the next case i think we can just update all the comments and it should be good uh the steps here the ones from falling from the following pr not the spec at the time of implementing this area okay so they removed the function and then we already did it the new way so um that can simply be removed i think and then let's quickly check yeah they use floor down here do we do the same um yeah okay so basically we already got the new thing um so we're just going to remove that comment let's see remove our updated comment from this one let's take the c plus bus name this is a normative change in the temporal spec i'm not going to need to run the test for this because obviously it's not the code change cool next one um editorial so i should probably explain the difference between normative and editorial um editorial is just um if it likes slightly changes the spec but it's not really noticeable in the implementation so you can't write the test that detects different behavior for example and normative is anything that uh changes observably behavior um so like in this case they removed something in this case they added another step um so this is probably just a tiny change yeah so it just moved something around basically but we're still gonna change that because um we like to have the implementation as close to the spec as possible otherwise it can get confusing so this is past temporal duration string this one and so what they do is take this and move it all the way to the end so let's see what it is um oh this might be the same okay um so this was probably added in the same pull request and so we already have that changed locally let's check out two comments okay yeah okay so we already did this as well so let's amend that comment a comment and just this is nano again add the second commit as well six just for reference and then move on so now this is one i did um a while ago and it just got matched so i believe i already have this fixed as well um which one is it test year oh yeah okay do we have a fixed me here we don't okay that's interesting uh usually i still um leave at least a fix me referring to the issue but it looks like in this case that's not the case um so it's changed from let test year be year plus one oh it's incorrect actually okay cool so and then was changed to that test here be test here plus one so we're just gonna use this and let's see if it fails the test no so it works okay cool um so what's the title here fix let's just copy that and then take the c plus function name this is a normative change put the comment in and let's go so it's just a bunch of small changes like this specifically but um many of them like have actual observable um incorrect behavior so it's good to get those fixed and in some cases also the string test 262 tests will currently fail if we implement it correctly um because they are developing based on the pulp fill which is sometimes already correct even though the spec is still wrong so this might actually fix a couple of tests as well um oh yeah this is interesting so what they did here was accidentally include underscores in a property name and so that obviously can be implemented like that and then it's uh not the correct or expected behavior so again what we did is um do it properly so we use vm the names for property names it's just an implementation detail but this is basically the string and it doesn't contain another score so we just did it correctly even better this spec said something different at the time so we'll just remove the text me um here as well and here as well it's in a couple of places i like that lots of fixed means grown that's nice and let's repeat it let's quickly run the tests sweet all working let's commit that again i'm taking the hash and that was titled fix the property name so fix smallest unit property name typo temporal uh typo in a couple of places okay like that and let's see the next one which is not mine but i think i reported that so yeah i know what the issue so basically this was just an indentation error so you could implement uh the function but it would do something different than intended just based on the fact that um it was not indented properly which is unfortunate but it got fixed so let's fix it as well in the implementation prepare partial temporal fields pre i can type this one um yeah so i also left the note that i believe this is uh the correct way but the spec has it differently so let's check that out here in the spec down here so we have b b1 b2 b3 b b1 um one two oh yeah okay right so it's already correct we have b3 and previously they had um a b and c as a third step but um yeah so that's basically just removing an outdated comment and let's also put the hash there so we can refer to it and commit that again no need to run the tests because it's just a simple comment removal moving on next one so we still got a couple to do normative make plain date difference method units handling consistent okay so let's check that out it's in this function which we already have because we have a complete basic implementation of the whole proposal um so at this point most of the functions that you will find in the spec are actually somewhere in the code and then what do they change um so they turn one step into two basically um presumably step eight one two three four five six seven eight yes and they change it into step eight and nine so we basically need to check this so in this case all the preceding numbers change by one which is uh just a side effect of inserting another step um that's fine let's just need to update all of these like this okay and this is the old one um so the old one called uh two largest temporal units options the siloed units auto and day and now they first want to do a different thing and then pass that instead of there okay that's doable so auto default unit is larger of two temporal units and we pass that the day and smallest units which i think we can probably safely dereference at this point um hmm i'm not sure we can actually so here's an optional string oh we have a fallback okay so in some cases um it returns optional string but um so it's either und in this back it's either undefined or a string but if we pass a fallback we can assume that it's never going to be optional so we can safely dereference it here and then we pass that instead of day and then this one is incorrect and let's see if that worked it did indeed so i think we're just gonna need the same somewhere else in since here okay check if it's the same largest units auto day default largest unit day smallest unit and then pass options this alert unit auto okay it's just the same so uh it's the largest unit change so this one can oh largest unit down here okay so this can all go and then we make increase every step by one and then let's quickly confirm that step 18 is actually the last one um in this back so just look that up and indeed it ends with 18. so we did that correctly and we want step 8 and 9. and i think we can just copy that from up here because it's identical and then let's run the tests again afterwards just to confirm yeah still passing okay so let's commit that um so what do they call it make plain date difference methods unique yeah i think we can just use the same commit message handling consistent this is a normative change in the temporal spec and is this commit and again it looks like a much larger change than it actually was because of updating all the comments next one thinks ambiguity and fractional part grammar okay so this is a change in the parser which is interesting because that was one of the most recent [Music] things that we added in temporal for a long time we were missing the iso parser which is now here uh looks like this pretty boring and all very similar but um it's functional so that's all i care about so what do they do they remove time fractional part they fully remove it yeah yeah okay it's no longer present so this will just be removed [Music] and then they change fraction which we have here to do not say decimal separator and time fractional part but just fractional part okay we can do that like that and then somewhere it's referred to presumably yes time fractional part so that becomes a fractional part and then they change something else okay it's the offsets because presumably fractional part includes the sign so plus or minus so they change it to start at one index further um so where is that and pass iso date time down here and it fraction is not undefined which is here so they change 0 or 3 to 1 4 and so on and so on so 1 4 7 and then okay so we just use a substring with length all three okay so that should be sufficient okay as you can see we got a a couple of failures here so i'm not sure why but let's look into it um so let's again check the spec change um so fraction has changed from time fractional part to fractional part and then we update updated all of this so let's check out the parser so what did we actually change here um oh okay so we forgot to update so we captured the result which is stored in the pass results struct and currently called time fractional part so that will need to be updated to just fractional pound because otherwise by default it's just empty and now it's no longer updated and then we actually need this change um down here now so this basically now captures whatever it matches in this which is the string view i believe yeah optional and then in the js function where it passes the different parts that is then taken and turned into a number yeah which is why the end bit here is zero yeah now it's working cool so we'll just take that again as well as the comet hash and that should be it yes okay uh next up okay i think this is also one i reported yes okay i wasn't really sure about this so i wrote a very long like text about why i think it's an issue and that got fixed which is nice so let's check that out so they use remainder here um but it's not supposed to so i think we already do it correctly and need to update the comment um yes so as you can see here now modulo there's like a small difference between remainder and modulo that can lead to subtle bugs i think in this case it was only for negative numbers something like that okay and for the um power i usually use this notation because here it is just raised but i can't really do that here um and that should be it uh don't really need to run the tests because i just updated the comment um spec comments in this function because again i just um i did it correctly the first time so now i just need to update the comment and that is it we are getting a lot done here next one normative add optional calendar to plane time plan year month and plain month day productions okay so this is again a parcel change i believe uh yes okay just shuffling a couple of things around and this is the polyfill so this doesn't affect us so let's check that out uh we got time here which i believe is fully removed yes okay so this can go away and then we oh we add calendar time okay so maybe i don't want to delete it i just want to move it next to time specs next to date time okay so this goes here and it's now called a calendar time oops so we're going to call it pass calendar time accordingly and it's gonna have a calendar time time spec time zone and optional calendar opt so the reason i use opt here is because that's how it's rendered in the spec like this okay so we have a calendar time i need to declare that and what else do we need so that's now being used down here for temporal month day string so instead of date time this now says calendar date time so we update both the spec string and um the function that's being used so it passes either a date oh that's oh it's in the different order here okay should write that comment and for temporal time string which is just below so instead of time date time this now passes calendar time and calendar daytime and so we have task calendar date time and pass calendar time just allowing a calendar presumably and then also in the month string down here which still takes a spec year month but no longer a date time but a calendar data yeah so these are all easy changes i'm just shuffling around a couple of things and this is in calendar string instead of time it says calendar time let's give that a try and it is working that is cool okay um oops let's just take the same message here optional calendar might not all fit on one line so let's just use this kind of syntax and play monthly um let's just say this adoption kinda um two plane time year month month day yeah it should be fine let's take the commit hash sometimes it's a bit limiting when you like have a limit and you kind of want to all fit it in there but it's fine fine like that don't need to overthink it okay so we got four more left to do this one is fix fix fractional seconds digits behavior in duration to string i'm not familiar what that means um okay so it's basically just an addition in temporal duration to string which is it's hard to tell like where in the whole thing it is because these all say one um so i kind of have to either ctrl f or just look around um this one okay step 21 which is here so let me close that actually uh step 21 which is down here and should now say this so it's just still the same condition or if precision is not auto so it's basically just a third or and then we need to check precision okay it's a variant so i'm not sure if it has a string at this point so i'm just gonna do has stringy and if it does then how do i get the string different get string view equals auto like this let's check where this is also used yeah okay down here it's also being checked so basically it's just the same as this um it needs to be included in the condition to reach this branch i guess let's run the tests okay we've got a lot of failures here um and just to make sure they are not related they are related to this change and not something i did mess up earlier i think i'll stash my changes here run it again okay yes actually that so um either we did something wrong or it just literally all changed um let's check the so in this case what i like to do is check the polyfill i'm just gonna look at this test um which is this one the plane times since smallest unit option failed so it's this one [Music] since as expected pt1h got pt1h0 seconds so i guess it's this one so let's just take these um so the polyfill can easily be tested by um visiting the temporal docs and then opening ajs console and it's just in there even if your browser or javascript engine does not yet implement tempora that's really convenient for testing so let's take this and this and then we have our for smallest unit so let's check that yeah this should return pt1h so i'm not sure why it doesn't let's check if we mess that up oh okay i'm silly if precision is not auto so this is a logic bug and i'm sure now all the tests will pass they do cool was worried there for a second okay let's commit that um let's just take this again and let's say pro 2 oh it doesn't fit so i say proto this is and we use us english spelling in the project so let's put the commit hash in there and we're good to go um next one so we got three more left let's just open them all in new tabs so allow t prefix and time only strings so this is once again a password change which interestingly they implement with regular expressions um so let's check this back text okay that's a small change um so we're getting rid of this production or rather renaming it i think so it's no longer called duration time designator but simply time designator duration time designators up here so let's rename that and then in calendar time which is obviously here wait why is that not here oh it is okay i just need to find it so that's no longer using time spec time zone but it's oh it's just adding an optional component so [Music] like this and the way that it works is just um okay now we need uh like a transaction thingy i think uh yeah right so this uh basically do does a state drawback because if this is present but um and you pause it and then this is not present you need to like go back between uh back to here so um we need a little bit of uh state management here and then we can just say um we so if it's optional we just ignore the result we pause it and if it fails we don't care and if it doesn't fail we don't care either basically so we just say pass time designator and then next up we pass a time spec which is not optional if that fails we return and since this is an raii object it will then roll back the state and its destructor and if we get to the end we just commit um so that's how this works uh what don't you oh yes okay let's run the tests here uh this does not work oh did i forget to rename something duration now where is that past time designated that should work so what don't you like pass duration time oh because it's renamed okay yeah that makes sense so up here it's no longer duration time designated but time designator so in duration time uh we just need to update this as well all of these and that's gonna be like this so it only does it once for all of them should be fine yes fantastic okay that worked so let's make another commit i think we can just steal that like this so next up disallow date only strings for playing time well that makes a lot of sense so presumably another parser change yes so um we're not going to remove it we're just going to add something new so instead of calendar daytime we now have a calendar daytime time required that is a very verbose name i like it color the date time time required let's declare that and then let's check how that works optional and that is also optional so we are gonna need a rollback for that um so for the very simple cases was like only one thing um if it doesn't pass and it didn't consume it so it doesn't need to roll back but for more um [Music] complicated expressions like this one we need state transaction so we pass a date if that fails we return then we pause a time spec separator if that fails we return and not optional and then we try to pass a time zone and we ignore the result of that because um it's optional so if it fails to pass that um it's just gonna try here basically um so we didn't consume it or if we did it did consume it then we also go to calendar and try to pass that so both of these are just ignored and i think that should work like this it did just work so let's make a commit this yeah that's a good good message um like this this is a normative change in the temporal speck and commit that okay and then last one um so this is uh pretty complex i saw and i wanted to kind of keep this video short so i think i may just do this off camera um it's just a bunch more passer changes adding a couple of new productions lots of very specific ones so i think we'll end it here if you made it this far then thank you very much for watching and i hope you found this interesting um definitely if you never saw the process of looking at the spec and updating it um i'm sure you now got an idea of how we do it and if you want to try it as well i'm sure there will be more changes um probably in january for temporal and maybe you can give it a try as well um yeah that's going to be it and see you next time bye

Channel: Linus Groh

Views: 522

Keywords:

Id: tazwznTJUq4

Length: 46min 56sec (2816 seconds)

Published: Sat Dec 18 2021