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

Add length as a property instead a method #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mgesmundo
Copy link

This small PR add length as a property instead a method, like the ES6 Map.
Thank you for your useful module!
All the best

@jhuckaby
Copy link
Owner

Thank you very much for this PR. It's a very clever trick. You'll have to forgive me, but since this module is already in active use in so many places (including many of my own projects) I really don't want to introduce a "breaking change" to the API like this. I have code that is "relying" on length() being a method, and changing it to a property would bust all that code.

You are absolutely correct here, and this is very much the way I SHOULD have designed it from the start. Your way matches the ES6 Map spec exactly, which I was trying to mostly emulate. I wish I knew about this nifty trick at the time.

I might revisit this for a future v2.0 rewrite, when I can freely break the API, but for v1.x, I have to keep it locked down.

But seriously, thank you for the contribution!

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