Mobx6 upgrade deps

[elektronik2k5]

Features:

• Upgrade all deps, except for @types/node cause I'm not sure which version of node is supported. If we have a policy I can set it up too.
• Remove fs-extra cause it wasn't used
• Run dedup script as part of prepare so any dep change is deduplicated automatically in yarn.lock
• Upgrade deps and package metadata of website, which removed yarn's warnings about missing license
• yarn perf is now faster: 45s instead of 60 on my machine 🏁
• All the tests pass, all the scripts run with one exception:
• test:codemod still fails with 2 snapshot errors, which was the case before my changes

Code change checklist

• Added/updated unit tests
• Updated changelog
• Updated /docs. For new functionality, at least API.md should be updated
• Verified that there is no significant performance drop (npm run perf)

Questions/Suggestions:

1. Since this is a maintenance PR, there's no need to update the changelog, right?
2. Do we have a supported node version policy? If so, should we enforce it? I can do that.
3. Which version of yarn should we support? If so, should we enforce it? I can do that. I can also make sure nobody runs npm by mistake.

[gitpod-io]

[danielkcz]

Prettier 2 has a different default for trailing commas. Can you please change the config back to "none" for now? And revert files that got modified because of that, please. If we are going to change formatting rules, it should be done in a single run.

[danielkcz]

Also, what version of Yarn do you have? I tried my version 1.22.0 and got a whole different output that's twice time bigger.

yarn.zip

[elektronik2k5]

Prettier 2 has a different default for trailing commas. Can you please change the config back to "none" for now? And revert files that got modified because of that, please. If we are going to change formatting rules, it should be done in a single run.

Done what I could. I'll explain: I added the two options which v2 changed the defaults for and applied those changes to the files in question. However, v2 also handles whitespace a bit differently, and that's NOT configurable - in the spirit of prettier :). So if we run yarn prettier now, we get this changeset:

	modified:   CHANGELOG.md
	modified:   codemod/undecorate.ts
	modified:   docs/best/actions.md
	modified:   docs/intro/concepts.md
	modified:   docs/refguide/api.md
	modified:   docs/refguide/boxed.md
	modified:   docs/refguide/extend-observable.md
	modified:   src/api/autorun.ts
	modified:   src/api/become-observed.ts
	modified:   src/api/computed.ts
	modified:   src/api/decorators.ts
	modified:   src/api/flow.ts
	modified:   src/api/makeObservable.ts
	modified:   src/core/globalstate.ts
	modified:   src/core/spy.ts
	modified:   src/types/legacyobservablearray.ts
	modified:   src/types/observablearray.ts
	modified:   src/utils/utils.ts
	modified:   test/perf/index.js
	modified:   test/perf/perf.js
	modified:   test/v4/base/action.js
	modified:   test/v4/base/array.js
	modified:   test/v4/base/autorun.js
	modified:   test/v4/base/autorunAsync.js
	modified:   test/v4/base/cycles.js
	modified:   test/v4/base/extendObservable.js
	modified:   test/v4/base/extras.js
	modified:   test/v4/base/makereactive.js
	modified:   test/v4/base/map.js
	modified:   test/v4/base/observables.js
	modified:   test/v4/base/observe.ts
	modified:   test/v4/base/set.js
	modified:   test/v4/base/strict-mode.js
	modified:   test/v4/base/tojs.js
	modified:   test/v4/base/trace.ts
	modified:   test/v4/utils/test-utils.js
	modified:   test/v5/base/action.js
	modified:   test/v5/base/api.js
	modified:   test/v5/base/array.js
	modified:   test/v5/base/autorun.js
	modified:   test/v5/base/autorunAsync.js
	modified:   test/v5/base/babel-decorators.js
	modified:   test/v5/base/babel-tests.js
	modified:   test/v5/base/cycles.js
	modified:   test/v5/base/decorate.js
	modified:   test/v5/base/errorhandling.js
	modified:   test/v5/base/extendObservable.js
	modified:   test/v5/base/extras.js
	modified:   test/v5/base/flow.js
	modified:   test/v5/base/makereactive.js
	modified:   test/v5/base/map.js
	modified:   test/v5/base/observables.js
	modified:   test/v5/base/set.js
	modified:   test/v5/base/strict-mode.js
	modified:   test/v5/base/tojs.js
	modified:   test/v5/base/trace.ts
	modified:   test/v5/base/typescript-decorators.ts
	modified:   test/v5/base/typescript-tests.ts
	modified:   test/v5/utils/test-utils.js

If you inspect the resulting diff, it comes down to 99% this single and unconfigurable change across all files:

-  computedProp: function() {
+  computedProp: function () {

So, prettier@2 adds a space between the function keyword and the parenthesis (). I don't think that's a big deal.
About 3 of those files have similar, insignificant whitespace changes which we similarly cannot modify in prettier@2.

We have two options:

1. Run prettier on the project and commit these as is. If it won't be me now then it'll be the next person to run yarn prettier.
2. I can configure prettier to run only for files staged for commit in a pre-commit hook using lint-staged, which we already have in the project but don't use for some reason.
   The result will be that each of these files will be reformatted lazily - at modification time, as part of the commit by the person who changes it - instead of right now by me.

From my experience, option 2 is better for applying any kind of static analysis enforcement in a collaborative environment. Incremental and granular is the only way it can work with minimal disruption to contributors and version control history.

[elektronik2k5]

Also, what version of Yarn do you have? I tried my version 1.22.0 and got a whole different output that's twice time bigger.

yarn.zip

I'm using the latest v1: 1.22.4. BTW, we can enforce yarn's version too in order to avoid such cases.

[danielkcz]

Thanks for the investigation. Let's not run Prettier for now to minimize the surface for conflicts with Michel's work. He can do it or it can be done later, no big deal.

Please configure git hook in the same way as Immer does. The lint-staged can be annoyingly slow with more changes.

I'm using the latest v1: 1.22.4. BTW, we can enforce yarn's version too in order to avoid such cases.

That's really surprising there would be that many differences between couple of patch versions. I would eventually like to switch it here to Yarn 2 which is installed inside the repo and removes the version hassle.

[Bnaya]

prettier 2 is an issue that better be handled after mobx6, or in stop-the-world manner by Micheal,
as it forces changes across the whole codebase

Also, run yarn-deduplicate on the lock files after the upgrades and yarn install again

[elektronik2k5]

@FredyC prettier is already setup just like that. Personally, I think the few seconds overhead which lint-staged adds are totally worth it because the immediate feedback is so valuable. I'd rather have my dev setup fail a commit which I'm now authoring instead of discovering it in hindsight in CI when I've already closed the task/computer and switched to another task.

I'm curious to hear mweststrate's feedback about my suggestions here.

@Bnaya that's exactly what I did:

• Run dedup script as part of prepare so any dep change is deduplicated automatically in yarn.lock

[danielkcz]

@elektronik2k5 I don't see a reason why to use slower lint-staged when https://www.npmjs.com/package/pretty-quick does the same but in a faster way. What feedback are you talking about? Prettier is an automatic tool, there is no intervention from your side necessary.

We don't need Michel's opinion here, please don't mention him with such trivial issues now. Prettier can be run anytime later, it doesn't matter. As I said, better to limit surface for conflicts.

[danielkcz]

Alright, looks like yarn.lock is correct now, merging then. Thanks.

[mweststrate]

In general, don't worry too much about conflicts on the mobx6 branch, I try to rebase often. Just don't change whitespacing and make sure file moves are committed correctly if applicable :-P.

…

On Mon, Apr 27, 2020 at 9:31 AM Daniel K. ***@***.***> wrote: Merged #2344 <#2344> into mobx6. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2344 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBGJDYA3M6E3FTMOPMLROU7FPANCNFSM4MRETWXA> .

[Bnaya]

prettier 2 will force some whitespace changes,
as not all of the changes are opt-out able via config