Make Id types mutually incompatible #5731
Draft
Conversation
This file contains hidden or 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
🎭 Playwright Test Results⏰ Completed at: 09/23/2025, 11:56:47 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
LinkSegment.id can be either a RerouteId or a LinkId. but guarding the type of instance as "not Reroute" or "not LLink" was not sufficient to narrow the type of id. It is split into an interface, and an instance type to ensure that any future changes create type errors instead of runtime errors. This also reverts the change to the createReroute code. The change was incorrect.
dosubot
bot
added
the
size:M
|
I'd rather brand them with unique symbols, if feasible. |
dosubot
bot
added
size:L
|
Agreed. That's a better approach. I'd be nice if there was a way to also have the "value" of the "brand" property symbol to be a unique symbol itself, but this didn't seem possible from testing. |
I would make a symbol adjacent to each type declaration instead of having the utility type. |
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.
NodeId, NodeExecutionId, and NodeLocatorId can all be strings. NodeId, LinkId, and RerouteId can all be numbers. As a result, Typescript will freely allow these values to be cast between one another.
To force these types to be mutually incompatible, they are defined as being extended from "number" and "string" with an additional optional type property. Actually extending primitives is a bad idea. Simply declaring types as extending primitives feels a little ick. Feedback on if such a change should actually be merged, or simply used as a temporary change for testing is appreciated.
Throughout the entire code base, only 1 actual issue was found, which was far fewer than expected: Left clicking on the center dot of a link prior to a reroute will only delete one link instead of all grouped links
┆Issue is synchronized with this Notion page by Unito