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

Strict immutability in NgRx 10 breaks Angular Material Datepicker #2690

Closed
livthomas opened this issue Aug 28, 2020 · 4 comments
Closed

Strict immutability in NgRx 10 breaks Angular Material Datepicker #2690

livthomas opened this issue Aug 28, 2020 · 4 comments

Comments

@livthomas
Copy link
Contributor

NgRx 10 with strict action/state immutability turned on (by default) doesn't work well with Moment.js and breaks for example Angular Material Datepicker with Moment.js adapter. It seems like NgRx freezes some objects that each Moment instance uses and Moment.js functions then throw errors. Notice that this happens even if you try to work with completely different Moment object from the one that is kept in the store.

Minimal reproduction of the bug/regression with instructions:

https://stackblitz.com/edit/ngrx-moment-bug

Open a date picker and select a date. See that the date was not selected and there is an error in the console:

Uncaught TypeError: Cannot add property l, object is not extensible
    at Locale.longDateFormat (moment.js:565)
    at replaceLongDateFormatTokens (moment.js:532)
    at String.replace (<anonymous>)
    at expandFormat (moment.js:537)
    at formatMoment (moment.js:521)
    at Moment.format (moment.js:3973)
    at MomentDateAdapter.format (material-moment-adapter.js:124)
    at MatDatepickerInput._formatValue (datepicker.js:3021)
    at SafeSubscriber._next (datepicker.js:2928)
    at SafeSubscriber.__tryOrUnsub (Subscriber.js:183)

Expected behavior:

NgRx 10 should work well with Moment.js even with strict immutability enabled.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

NgRx 10, Angular 10, Angular Material 10.1, Moment.js 2.27

Other information:

This worked without any issues in NgRx 9 even with strict immutability turned on.

I would be willing to submit a PR to fix this issue

[ ] Yes (Assistance is provided if you need help submitting a pull request)
[x] No

@timdeschryver
Copy link
Member

Thanks for including an example, this really helps!
I think this is because the behavior something material-moment related is changed.
When I downgrade to NgRx 9.2, the error remains.

What I currently don't understand is that how selecting a date can cause this issue (because it's just a stand-alone input field that isn't bound to the state)

@livthomas
Copy link
Contributor Author

@timdeschryver Interesting, I'm not able to make it work now with previous versions of Angular and NgRx. But it was working in my application before I updated to Angular 10 and NgRx 10.

I believe this problem is not directly related to Angular Material library. The first time I ran into this error was when I tried to use Moment.js functions directly. Only later I discovered it didn't work with Angular Material Datepicker either. So I used it in my reproducer but I think you should look for a problem in Moment.js instead.

@livthomas
Copy link
Contributor Author

@timdeschryver By the way, other state management libraries ran into this problem too: salesforce/akita#124

@timdeschryver
Copy link
Member

@livthomas you're right, this has nothing to do with Material, but with Moment.
I'm not sure but I think moment updates the moment instance in the store after x seconds.

This isn't something NgRx can solve (as it's also pointed out in the akita issue) and I would recommend to store primitives inside the Global NgRx Store.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants