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

Move unwrapping methods to the primary underscore.js module #2860

Merged
merged 2 commits into from
Jul 6, 2020

Conversation

jgonggrijp
Copy link
Collaborator

While writing an article on how to use the upcoming modular Underscore release, I realized that the division between the underscore.js and underscore-oop.js modules that I made in #2849 was a bit unnatural. As it was, the underscore.js module exported a _ function that could wrap a value, but the resulting wrapper had no unwrapping methods. To have the unwrapping methods, you needed to import underscore-oop.js for its side effects, but that also added all the array proxy methods. People who cherry-pick from the library may sometimes want to have wrapping and unwrapping but no array methods.

I reorganized this slightly so that underscore.js already packs the unwrapping methods. The remaining underscore-oop.js has only one job, i.e., to add the array methods, so I renamed it accordingly. The impact on the monolithic build is minimal because the unwrapping methods have no dependencies of their own.

I don't expect anyone to object to this change so I'm not going to ask for a review, but I'm creating this PR anyway in order to leave a trace. I will also hold off from merging until the Travis build has completed, to make sure that nothing breaks.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.263% when pulling a2c9f6d on jgonggrijp:underscore-unwrap into faac06d on jashkenas:master.

@jgonggrijp jgonggrijp merged commit e6af0f9 into jashkenas:master Jul 6, 2020
@jgonggrijp jgonggrijp deleted the underscore-unwrap branch July 6, 2020 18:54
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