Open 43081j opened 3 years ago
no worries, i'll try get it sorted in prettier's repo too.
really its just trying to cache the plugins list/loading between multiple calls. honestly i doubt a dependency is even needed in reality, prettier should have kept the plugins in a cache/context themselves and only called the function to populate it once IMO. instead of relying on caching a function's result.
anyway ill go sort it out in prettier and update this afterwards
no worries, i'll try get it sorted in prettier's repo too.
I see https://github.com/prettier/prettier/pull/11230, thanks!
One comment I have from description in https://github.com/prettier/prettier/pull/11230:
or we could just throw a
WeakMap
together ourselves
It makes me start to wonder if it would make sense to consider using macros ... see https://github.com/brodybits/ask-me-anything/issues/35
OK it looks like the upstream work moved into https://github.com/prettier/prettier/pull/11241 and has not moved forward as smoothly as I had hoped and expected. I probably should have explained more clearly that upstream Prettier uses their build step to publish with all dependencies included.
I think it would be helpful for both upstream Prettier and this fork if we could get a better understanding of the motivation behind cleaning up this one dependency for the caching. My impression is that the "bloat" of the existing mem
dependency, as bad as it is, would be relatively minor in comparison to the number of dependencies and sub-dependencies that are used to support the other parts of Prettier(X).
More important to me (at least) is ease of understanding. I am very sorry to say that I have not found any of the proposals to be very easy to understand. I think it would be ideal if we could find a way to make the cached config mechanisms both less bloated and easier to understand at the same time.
Another topic which I think belongs in a separate discussion is the effect of publishing to npm directly vs publishing from a build. Publishing from a build like Prettier does makes installation a little easier, seems to fix ESM support, and supports the browser more directly. Disadvantages would include need for explicit license statement for bundled dependencies and hiding of vulnerabilities such as possible REGEX-DOS in bundled dependencies.
the mem dependency pulls in a few levels deep worth of dependencies for something extremely simple.
here i've replaced it with fast-memoize which has no dependencies and seems much faster/simpler.
if the
memoizeOptions
repetition puts you off, we can probably just change it tocreate() { return memoizeCache; }
since a map coincidentally follows the same interface fast-memoize expects.let me know if there's any changes you want, feel free to let me know if you're not interested in this change too