Skip to content
This repository has been archived by the owner on Sep 21, 2021. It is now read-only.

Update rxjs dependency version #33

Merged
merged 1 commit into from
Aug 5, 2017

Conversation

IanVS
Copy link
Contributor

@IanVS IanVS commented Aug 5, 2017

As a helper library adding a non-trivial dependency into consuming projects, I think it make sense to be as liberal with the version range as possible. Meaning, it seems like this package should specify the minimum version that will work with office-js-helpers.

This would mean that apps using it which are depending on (and perhaps locked with yarn.lock or package.lock) for example version 5.1.0 shouldn't necessarily need to also ship version 5.4.2 to all of their users just because of a mismatch with this helper library. As an example, this PR saves 7.88kb in my own project. Which granted isn't huge, but it's not nothing either, and slow load times in task panes is the worst. :)

From what I can tell, this package is just using a pretty vanilla observable and doesn't seem to be doing anything too crazy with it, so this PR just specifies that rxjs: ^5.0.0 should be installed, while still updating the yarn.lock file so that developers working on this project get the latest version.

What do you think?

@msftclas
Copy link

msftclas commented Aug 5, 2017

@IanVS,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

Copy link
Contributor

@WrathOfZombies WrathOfZombies left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the PR.

@WrathOfZombies
Copy link
Contributor

@IanVS, please complete the CLA agreement and I can merge the pull request.

@msftclas
Copy link

msftclas commented Aug 5, 2017

@IanVS, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@IanVS
Copy link
Contributor Author

IanVS commented Aug 5, 2017

Signed! 👍

@WrathOfZombies WrathOfZombies merged commit 44d3e81 into OfficeDev:master Aug 5, 2017
@IanVS IanVS deleted the 32-upgrade-rxjs branch August 6, 2017 17:19
@IanVS IanVS mentioned this pull request Aug 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants