Fix some param reassignments, and enable lint. #72
Conversation
There was a problem hiding this comment.
This is great! Such a great idea to do this. Every file except ecmascript.ts should be fair game for this change, and I'll plan to do the same for ecmascript.ts.
Do you think we should make the same change to the proposal-temporal code to keep them in sync? Or best to make it only here? @ptomato do you have an opinion?
|
I think an advantage of porting this to proposal-temporal would be reduced divergence for porting future changes from proposal-temporal to here, but on the other hand it would increase the divergence between the proposal-temporal code and the specification. So I feel neutral-to-negative about that. |
|
(Not a Temporal Champion but) I'm in agreement with Philip here - the spec isn't written this way (though, frankly that seems a bit odd to me too, though I suppose for JS itself it doesn't really care about reassignment in the way that developers do for e.g. readability of the code), but given that it is I think it makes sense to leave the proposal repo as-is. |
12wrigja
force-pushed
the
fix_param_reassignments
branch
3 times, most recently
from
395e826 to
0ecdc3f
Compare
12wrigja
force-pushed
the
fix_param_reassignments
branch
from
0ecdc3f to
e53546a
Compare
|
Ok - I've fixed up the rest of the files besides ecmascript, and adopted the pattern that Justin suggested (which lead to some much nicer diffs!). PTAL. |
|
@justingrant sounds great. I've just "finished" (tests all pass) integrating JSBI into Temporal (see #77), and I want to rebase that ontop of your ecmascript changes and have fixed the lint suppression for ecmascript.ts before I send that for review. |
Finishing what js-temporal#72 started. Previous commits avoided mutation of params only where the types changed. This commit adds eslint support for preventing param mutation and makes changes that don't change the underlying type. I split into a separate commit in case we want to make this a new PR.
Finishing what js-temporal#72 started. Previous commits avoided mutation of params only where the types changed. This commit adds eslint support for preventing param mutation and makes changes that don't change the underlying type. I split into a separate commit in case we want to make this a new PR.
Fixed for PlainDate, PlainDateTime, PlainMonthDay, PlainTime, and PlainYearMonth so as not to clobber @justingrant's work on fixing up types for the other files (not sure how cleanly the PRs would merge).