Skip to content

Conversation

@crudh
Copy link
Contributor

@crudh crudh commented Jun 16, 2020

MomentUtils.formatByString takes the date param and changes it by setting a locale.

Everywhere else in the file clone is used but not here. So it's an unexpected mutation of a passed param and it blows up if date is frozen.

The only changes are in the formatByString, the rest was applied by husky on commit.

Comment on lines +148 to +154
const clonedDate = date.clone();
clonedDate.locale(this.locale);
return clonedDate.format(formatString);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual change.

Copy link
Owner

@dmtrKovalenko dmtrKovalenko left a comment

Choose a reason for hiding this comment

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

Sorry for long response, I was on vacation.

Thanks for the change, but it looks like your local prettier configuration conflicts with our.
Could you please run prettier on all files yarn prettier --write './src/**/*.{js,jsx,ts,tsx,json,css,md,mdx}'

@crudh crudh force-pushed the fix-format-mutation branch from 8075acd to 6d05b40 Compare June 19, 2020 19:30
@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2020

Codecov Report

Merging #394 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #394   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          552       553    +1     
  Branches        64        64           
=========================================
+ Hits           552       553    +1     
Impacted Files Coverage Δ
packages/moment/src/moment-utils.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 400e9f6...6d05b40. Read the comment docs.

@crudh
Copy link
Contributor Author

crudh commented Jun 19, 2020

@dmtrKovalenko fixed now!

Seems like the husky/lint-staged rules runs prettier with the global configuration instead of the local one? It did the reformatting earlier. I disabled it temporarily and then everything was fine.

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.

3 participants