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

feat: support EsModules #143

Closed
wants to merge 1 commit into from
Closed

feat: support EsModules #143

wants to merge 1 commit into from

Conversation

aminya
Copy link

@aminya aminya commented Sep 1, 2024

This adds support for EsModules to Memoizee. This is a step towards the modernization of this library. The old Node 0.12 support is maintained via Babel, and thus these changes are fully backwards compatible.

When ' ESM ' is used, the main memoizee function loads the extensions asynchronously via dynamic imports. This helps prevent the bundling of unused code in the browsers.

31st Aug 2024 benchmarks:

1:    13ms  Memoizee (object mode)
2:     2ms  Memoizee (primitive mode)
3:     2ms  Underscore
4:     3ms  Lo-dash
5:     7ms  Memoizee (primitive mode) LRU (max: 1000)
6:     9ms  Memoizee (object mode)    LRU (max: 1000)
7:     5ms  lru-cache                 LRU (max: 1000)
8:     5ms  secondary-cache           LRU (max: 1000)

@aminya aminya force-pushed the Modernize branch 8 times, most recently from 9c83302 to fc75695 Compare September 1, 2024 03:05
@aminya aminya marked this pull request as ready for review September 1, 2024 03:06
aminya added a commit to aminya/setup-cpp that referenced this pull request Sep 1, 2024
aminya added a commit to aminya/setup-cpp that referenced this pull request Sep 1, 2024
aminya added a commit to aminya/setup-cpp that referenced this pull request Sep 4, 2024
aminya added a commit to aminya/setup-cpp that referenced this pull request Sep 4, 2024
Comment on lines +12 to +23
if (length === false) {
options.normalizer = (await import("./normalizers/primitive.mjs")).default;
} else if (length > 1) {
options.normalizer = (
await import("./normalizers/get-primitive-fixed.mjs")
).default(length);
}
} else if (length === false)
options.normalizer = (await import("./normalizers/get.mjs")).default();
else if (length === 1)
options.normalizer = (await import("./normalizers/get-1.mjs")).default();
else options.normalizer = (await import("./normalizers/get-fixed.mjs")).default(length);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These modules are tiny, and there's not much benefit in importing them lazily. I can import them at the top-level instead of using dynamic imports. This would make the memoize function sync again similar to the cjs implementation.

@medikoo
Copy link
Owner

medikoo commented Sep 9, 2024

@aminya great thanks for the effort, but at this point I do not plan to refactor this package into ES modules.

I believe this package is also perfectly accessible from ES modules context.

Ideally such changes, before taking implementation effort should be first consulted on issue level

@medikoo medikoo closed this Sep 9, 2024
@aminya
Copy link
Author

aminya commented Sep 9, 2024

No problem. I assumed that this repo will not be open to modernization given the number of polyfills that are imported directly. I'll create a hard fork then.

@aminya
Copy link
Author

aminya commented Sep 10, 2024

Here's the fork link for those interested. I'll probably rename and publish the modernized package on npm soon. It will be API-compatible with memoizee, and will support all the old platforms via Babel.
https://github.com/aminya/memoizee/

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

Successfully merging this pull request may close these issues.

2 participants