Skip to content
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

Structural setup around now forbidden undefined and null values (since v8) #308

Closed
holgerd77 opened this issue Jun 21, 2023 · 2 comments
Closed

Comments

@holgerd77
Copy link

holgerd77 commented Jun 21, 2023

Hi there,
Holger from EthereumJS https://github.com/ethereumjs/ethereumjs-monorepo, we use the lru-cache package for 3-4 cache implementations, thanks for that! 🙏 🙂

I am right now updating from v7 to v9 or v10 and have some structural question around undefined and null values: so we used to use undefined as a cache value to indicate that we know that the respective value (e.g. for a trie datastructure node) is not present in the underlying database (so this undefined value was actually "carrying some information").

Since undefined and null are not allowed any more since v8: is there any best recommended practice how to "upgrade" on such a situation the most clean way? Or is such a setup just no been considered along the upgrade, and was just somewhat forgotten?

For us I have now "solved" the situation by adding a @ts-ignore on the constructor, since from my feeling the usage of undefined is still the cleanest solution for our use case.

@isaacs
Copy link
Owner

isaacs commented Jun 21, 2023

Yeah, rewriting in typescript exposed a conflict in how fetch() was possibly handling undefined values in the case of aborted fetches, unfortunately. It's a logical consequence of supporting ignoreFetchAbort and allowStaleOnFetchAbort, meaning that aborts aren't necessarily a failure, and I missed the implication when those features for added. I see this as ultimately my error, and typescript doing its job quite well, but still, annoying lol.

The clearest way to approach it imo is to have a special "no value here" value, like Symbol('undefined'), or wrap your values in an object like type Value = { value: MyThing | undefined }. It does add a step in the cache lookup, but it also means that cache.get(key) will either return undefined (not in cache) or {value:undefined} (known to be missing).

@holgerd77
Copy link
Author

👍

Thanks a lot for going so deep with your answer, yes, we already had this wrapping solution in parts, but since I thought about this whole thing a couple of rounds already I finally wanted to loot out a bit if this all things are structurally intended/necessary/whatever. 🙂

Generally, the whole upgrade to the TypeScript/non-default-export/whateer new versions (we upgraded from v7) worked pretty seamlessly.

@isaacs isaacs closed this as completed Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants