Repository navigation
Conversation
NepipenkoIgor
left a comment
There was a problem hiding this comment.
Thanks for the fix, @endlacer! Verified locally on top of develop: the full lib suite is green (905 tests) and 4 of the 5 new tests fail against the old pipe, so they do cover #1669. A per-pipe NgxMaskService with patterns merged from the provided defaults is the right approach.
Two requests before merge:
-
Please move the tests into
mask.pipe.spec.tsas a nesteddescribe('config isolation (#1669)')and dropissue-1669.spec.ts. Pipe regressions belong with the other pipe tests (#1567 and #1492 are already there), and that file has the TestBed setup thatcreatePipeduplicates. The existingissue-NNNN.spec.tsfiles are a pattern we want to move away from, so please don't add to it. -
One leak of the same kind remains on a single pipe instance:
pipe.transform('12', '00'); // '12' pipe.transform('12', '00', { patterns: { '0': { pattern: /[a-z]/ } } }); // '12', expected ''
applyMask(ngx-mask.service.ts:311-322) still seescurrentValuefrom the previous call and returns the input unmasked. It pre-dates this PR, so say if you'd rather leave it for a follow-up, but resetting the per-call service state at the top oftransformplus a test would close the issue properly.
No need to touch the version or CHANGELOG.md, we'll handle that at release.
fce4bbd to
6412554
Compare
fixes 1669