chore(zones): update zones/proxies to use connectedSubscription #2128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: John Cowen <john.cowen@konghq.com>
✅ Deploy Preview for kuma-gui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
kleinfreund
approved these changes
Feb 5, 2024
johncowen
added a commit
that referenced
this pull request
Mar 14, 2024
Now Typescript comes with built-in declarations for `Object.groupBy` its a little less annoying to polyfill use Javascript's `Object.groupBy`, also see the description in #2128. This PR adds a polyfill and uses it an a couple of places where conversations have arisen lately. most of the change here is as a result of a re-nest, but the code that came up in past conversations is now just a one-liner. There are a few places I'll be switching this in at some point (including the code in above linked PR) I tested the polyfill manually in Firefox 118. Signed-off-by: John Cowen <john.cowen@konghq.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simplifies the 'knitting' of Zones and their Proxies by using connectedSubscription.
I think this is the last place where we had a subscription inspection without using our new
Subscriptiondata layer things. See #1890Separately, I spent a lot of time trying to get typescript to understand
Object.groupByand/or a type safe polyfill for it, which would simplify this even more and use even less of our own code, but I kinda gave up trying. I can come back to that at a later date. edit: I didn't PR this branch straight away for reasons, and luckily I found out todayObject.groupByis coming to typescript in the next release in around a month or so, so I'll wait for that. I still have a branch or stash here with the nativeObject.groupByusage, so I can PR that once the support comes with typescript.Also, its important for me to make sure I note in relation to #1918 (comment) that I've removed an instance of
isSetusage whilst I was here. For the following reason: Unless I've missed something,withConfighere can never benull(its the result ofArray.findwhich never returnsnull) so I don't see any point usingisSetwhere we can just use straight/native JS instead i.e. this is the better refactor/fix.There are other places where I think we no longer need
isSet, but seeing as I was changing code here anyway, I only updated things here for the moment.