nest
nest copied to clipboard
Revert "Revert "fix: update factory to account for old v4 syntax for cache stores""
This reverts commit 530af92.
PR Checklist
Please check if your PR fulfills the following requirements:
- [X] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [X] Tests for the changes have been added (for bug fixes / features)
- [X] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
- [X] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:
What is the current behavior?
Reverting a revert, I know, always fun! The flakiness of the tests came from how v5 uses milliseconds whereas v4 uses seconds to the ttl. This caused our tests to not pass because the HTTP calls weren't quite fast enough all of the time.
What is the new behavior?
I updated the timing of the tests to be 100seconds or 100 milliseconds depending on the cache-manager version and that seems to have stabilized the test. The other option we have here would be to automatically multiply the ttl given to the CacheModule.register/Async() by 1000 to convert the ms to s. This would make other user's transition from v4 to v5 smoother, however, if someone were to put in a seconds value already then we've taken it from seconds to a large number of minutes, which would cause the cache to work incorrectly for their use case. It's a tricky one to be sure. Happy to go with either approach though.
Does this PR introduce a breaking change?
- [ ] Yes
- [X] No
If anything is a breaking change it's that the ttl is now in milliseconds instead of seconds like it was in v4, but that's a difference of that package and not with out implementation.
Other information
We should update the @nestjs/cli's default webpack to include cache-manager/package.json in the excluded packages
Pull Request Test Coverage Report for Build da80898b-2ef8-4ef0-85a6-3ca23b7c028c
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 93.786%
| Totals | |
|---|---|
| Change from base Build 6b637f64-9c7a-4d7b-8537-b6a8d098f58e: | 0.0% |
| Covered Lines: | 6113 |
| Relevant Lines: | 6518 |
💛 - Coveralls
lgtm