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: add support for importing functions individually #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danazkari
Copy link

This PR adds support for not only still being able to import the entire library and just assign memory pointers to the functions that the user of the API wants to use, but also adds support so that said user can import each individual function in case they're looking for performance wins on their projects.

@prantlf
Copy link
Owner

prantlf commented Jul 3, 2019

This change uses a clever way to keep compatibility with the named exports, in comparison with #8. It would allow some "deprecation time", so that people could get a friendly warning by an eslint plugin, for example and plan the switch, until the major version changes and they will have to.

However, I'm still missing the benefit of this, as I commented on the original PR.

@danazkari
Copy link
Author

Right, well perhaps I'm the one looking at this from the wrong angle.
Doing import { someFunction } from 'full-package'; it assigns whatever full-package exports with the name someFunction to a memory pointer using that same name. That's a very clever way of doing things, but, even if they're not available in the context, the rest of the package was also imported.

While it's true that tree-shaking would take care of unused code, if a project doesn't use tree-shaking at all and they just need a specific function, this adds a lot of code that's not gonna be used and increases package size.

The other reason I see is, date-fns specifically has this same pattern where you can just import the format function for example and ignore the rest of the code. This is the main reason why we're using it in our project to get rid of momentjs. I honestly when I started using this project, expected it to behave the same way as date-fns and was in for a surprise 😛

PD: I know I messed up the tests, I'll correct them as soon as I get a chance today.

@houmark
Copy link

houmark commented Jul 15, 2019

Would love to see this merged!

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.

3 participants