eslint: missing-observer, no-anonymous-observer

[urugator]

See README.md

[changeset-bot]

🦋 Changeset detected

Latest commit: c26daa2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
eslint-plugin-mobx	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[kubk]

@urugator Thank you for the implementation. I've gathered some feedback from people who use Mobx. The main question is can I wrap all react components with observer without getting performance overhead? I've only found this in the docs: https://mobx.js.org/react-integration.html#always-read-observables-inside-observer-components

[urugator]

We also suggest to dereference the values as late as possible, which practically means that all components must be observer. So observer should better be cheap otherwise it doesn't make much sense.
I would ask the other way around, how many components in your code base don't have to be observer? If it's a significant portion, then I would argue you don't use mobx idiomatically. If it's insignificant, then there is no issue, because the potential overhead is most likely insignificant as well. You will always have exceptions, but imo it's better to threat these, well, "exceptionally", rather than making everything else more complicated.
I would definitely start with just slap observer everywhere and if it wouldn't work (like fundamentally), then probably I wouldn't use mobx, because the selling point is that you don't have to think about and manage the subscriptions.

[urugator]

Added no-anonymous-observer and changed both to warn by default. Basically I want to force people to try it out and provide feedback. Any opinions?

[iChenLei]

Hi, Mr.urugator. I think you can add eslint-plugin-mobx to mobx.js.org, pin a beta flag. You should do more promotion (twitter, blog or whatever), no users, no feedback. @urugator

[kubk]

@urugator Once the plugin is released, I'll promote it among our Mobx group in Telegram (around 300 people). Since it's not merged yet, I can't try it on our projects.

[urugator]

Good lord, I had to recreate yarn.lock and now I am getting typescript errors all over the place absolutely unrelated to this PR.

[urugator]

Not compatible:

{ observableKind: "map"; debugObjectName: string; } & { object: ObservableMap<K, V>;             name: K;       type: "delete"; oldValue: V; }
{ observableKind: "map"; debugObjectName: string; } & { object: ObservableMap<unknown, unknown>; name: unknown; type: "delete"; oldValue: unknown; }

Any ideas?

 The types of 'object.data_' are incompatible between these types.
        Type 'Map<K, ObservableValue<V>>' is not assignable to type 'Map<unknown, ObservableValue<unknown>>'.
          Type 'ObservableValue<V>' is not assignable to type 'ObservableValue<unknown>'.
            Types of property 'enhancer' are incompatible.
              Type 'IEnhancer<V>' is not assignable to type 'IEnhancer<unknown>'.
                Types of parameters 'newValue' and 'newValue' are incompatible.
                  Type 'unknown' is not assignable to type 'V'.
                    'V' could be instantiated with an arbitrary type which could be unrelated to 'unknown'.

[kubk]

@urugator Have you accidentally updated TS version? Also as far I remember Mobx never used TS strict mode internally. Maybe it was accidentally enabled.

[urugator]

main has the same TS version in package.json as I do locally (^4.0.2) , but I think the yest.lock on main is outdated, so it actually uses older version: https://github.com/mobxjs/mobx/blob/main/yarn.lock#L13419

[urugator]

I am starting to loose my temper. Yesterday I went through all packages to solve/workaround compat with TS4. Run tests in each, all good, push to server, nope error on CI. Today I've rebuild eslint-plugin-mobx, run tests, same error, that's good, because at least it's replicable. Installed jest:27 in eslint-mobx-plugin, error is gone and guess what, it breaks tests in all other packages...

[mweststrate]

@urugator let me know if you want me to take a look into it. I haven't followed the discussions, but if you can tell me what steps fail I can check it out

[urugator]

Seems like there is a compatibility issue between eslint:8 and jest:25/26 (dunno which, because we have 26 in deps, but tsdx uses 25). For the moment I just downgraded eslint to 7 and I will call it a day.
However I would love to understand why installing deps in one package affects other packages. I feel like I am missing something about this monorepo setup.
I would also like to know if there is a difference when I run a command like yarn test from individual package and when I run it from root - does it use the same version of jest for all packages or can a package use it's own jest (or typescritp etc) version?
Also we should get rid of TSDX as it's not maintained, we are stuck with the deps and imo it just obfuscates the whole setup.