-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
What's not production ready about this? #107
Comments
We're using it in production and no issues so far |
This is a polyfill for a TC39 proposal that is still in stage 2, and thus it could significantly change before being shipped in browsers. |
@nicolo-ribaudo ahh I see. Btw is the proposal dead? I've started to really love records and tuples in its current form, they solve a lot of problems. But since this repo hasn't been changed in 2 years and the proposal hasn't in 8 months, it's hard to feel hope? I'm totally fine with the current state, the only thing that's annoying is converting a backend response that contains nested objects to a structure with records/tuples. I had to write a function that recursively makes things records, would be nice if there was a built-in way. So if this could be merged into JavaScript and we'd get proper TypeScript support that would be amazing 🤞 |
Performance is likely to be the largest one. See #59. The implementation takes a general approach to all R&T - focusing on correctness rather than speed. |
Hi @danieltroger is this usage in a public repo? I'd love to have a look to see how R&T are helping |
So I don't think the implementation could be much better optimized. At some point whether you use Immutable or whatever else you still have to look at every key-value in the worst case scenario. This like the other libraries has a strategy for early exiting. If your anything like me you use a validator like Zod so you'll have to do a full pass on the data anyway so what's two passes? At least with an immutable data structure you do the pass once rather than littered everywhere throughout the code base. |
@acutmore: No, unfortunately not in a public repo :/ But we do have public source maps that you can use to read all the code :)
Basically that one and DragAndDropZones.tsx besides it is where we get the most benefits from R&T. ![]() We're also using it in this other file, but it's contested (the whole logic is). We're trying to diff local state vs server state there. To know if the user made any changes. It's fragile because false positives might come from random stuff that's in the state that we don't care about, and if we would revert to our pre-R&T approach where one manually compares all interesting properties false negatives are possible as things are added to the state but the comparison logic isn't updated. Seems like it works quite well now so it might survive for a while more but the "less-complexity" approach is to just count any change as "a change was made" (disadvantage that we might indicate changes were made when they weren't) ![]() |
As far as I can see, this is well tested and well written. What about this isn't ready yet?
The text was updated successfully, but these errors were encountered: