Expose and document the MVVM observer boundary#2311
Closed
woksin wants to merge 3 commits into
Closed
Conversation
withViewModel observes only the render it wraps, so a child that reads observable view model state directly needs its own observer boundary. Expose observer from the @cratis/arc.react.mvvm package root with documentation, so consumers wrap a leaf with it instead of importing from mobx-react/mobx-react-lite directly, and cover the export with a spec.
Flag direct imports from mobx-react and mobx-react-lite and steer consumers to import observer from @cratis/arc.react.mvvm instead, keeping the MobX binding an internal detail. Register the rule in the recommended config and cover it with rule tests.
Explain that withViewModel observes only the component it wraps, recommend passing plain props to presentational children, show leaf-only observer() for children that must read view model observables directly, and discourage blanket wrapping and direct mobx-react imports.
|
NuGet packages for this PR, e.g. Cratis.Arc: |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the sanctioned MobX observer boundary in Cratis Arc React MVVM explicit, documented, and guarded, so consumer apps stop reaching through the transitive MobX dependency or blanket-wrapping components.
Added
observerfrom@cratis/arc.react.mvvmso a child component that reads view model observables directly can be given its own observer boundary, with guidance in the React MVVM documentation.no-direct-mobx-react-importESLint rule that flags direct imports frommobx-react/mobx-react-liteand points consumers to@cratis/arc.react.mvvminstead.Changed
mobx-react/mobx-react-liteimports as errors.