Skip to content

Conversation

@AndrewADev
Copy link

Adds changes to export the alias of change types supported by deepObserve with a slightly less generic name.

Also adds more detail to the relevant doc comments.

Export

The alias used by MobX-utils for the change parameter of deepObserve, previously named IChange, is not currently exported.

It only consists of types exported by MobX, though. So, when setting up a listener in an application importing mobx-utils, I can define something along these lines to get type information for my listener:

export type IDeepDidChange =
  | IObjectDidChange
  | IArrayDidChange
  | IMapDidChange;

Nonetheless, the exact set of supported types of changes could of course change in the future (such as once #298 addressed). So, exporting this alias directly from MobX-utils would provide up-to-date type information directly from the package itself.

Renaming - Rationale

The thinking for renaming IChange to IDeepDidChange is influenced by:

  • Deep - I thought perhaps it would be good to have something not quite as generic as simply 'change' to reflect the fact that this is coming from MobX-utils, and that there are different types of changes are supported in deepObserve than in MobX core right now (seen in deepObserve not working with ObservableSet #298)
  • What seems to be an existing convention for 'WillChange' vs 'DidChange'

That said, I'm open to a different name than the one proposed, if there is something that better matches MobX conventions or I'm simply reading too much into the conventions here.

Documentation

I'm assuming I should just change the JSDoc comment, as it seems the README is auto-generated before a release? Let me know if that is not the case or I need to update the README as well.

Beyond that, I tried also documenting the callback more thoroughly via a separate JSDoc block, like so:

/**
 * Callback invoked when a deeply observed change is detected.
 *
 * @callback onDeepChange
 * @param {IDeepDidChange} change The type of change
 * @param {string} path The path to the property where the change occurred
 * @param {Object} root The root observed object
 */

Which was valid for generation (I tested with yarn run build-docs), but the callback was then given the same level of nesting as deepObserve (and others) in the README instead of being nested under it (presumably what we'd want). It looks like there is possibly a way to get documentation to do that, though? Let me know if you see value in pursuing that (also for elsewhere)

Export the type alias that contains information on the types of changes currently supported by `deepObserve`.

Also rename it to be more specific and reduce the risk that it is mistaken for something coming from the main MobX library.
Document types and descriptions, extend example.
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.

1 participant