Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

mobx-react v6 #644

Merged
merged 49 commits into from
May 29, 2019
Merged

mobx-react v6 #644

merged 49 commits into from
May 29, 2019

Conversation

mweststrate
Copy link
Member

See #640

src/Provider.js Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/Provider.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/observer.js Show resolved Hide resolved
src/observer.js Outdated Show resolved Hide resolved
src/observer.js Outdated Show resolved Hide resolved
@mweststrate
Copy link
Member Author

Besides processing a lot of TODO's, the biggest open question atm is how to integrate with the DevTools.

package.json Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-Authored-By: mweststrate <mweststrate@gmail.com>
CHANGELOG.md Outdated Show resolved Hide resolved
@mweststrate
Copy link
Member Author

mobx-react@6.0.0-rc.2 is now available!

@samcooke98
Copy link
Contributor

samcooke98 commented Mar 21, 2019

mobx-react@6.0.0-rc.2 is now available!

Just a heads-up, it seems that it breaks when attempting to use observer(someClassComponent)

https://codesandbox.io/s/j3k4p2jkv5

I did some digging, it seems h is the built <Observer> from mobx-react-lite - Not sure why it's not included in the bundle however?

@danielkcz
Copy link
Contributor

danielkcz commented Mar 22, 2019

The h is not the Observer, it should be React.createElement, but it's indeed missing in there for some reason. The Observer is minified under y and is there available.

@mweststrate I don't think you should be providing a minified code as a main, that should be a job for the consumer and that's probably a reason why external react is not referenced correctly.

@mweststrate
Copy link
Member Author

mweststrate commented Mar 22, 2019 via email

@xaviergonz
Copy link
Contributor

Are you using terser rather than uglify es? Terser fixed some react minification bugs

@mweststrate
Copy link
Member Author

mweststrate commented Mar 22, 2019 via email

@danielkcz
Copy link
Contributor

So I think you should remove reexport of useDisposable, useComputed and useObservable as I came to the conclusion these should be removed from lite package anyway. Let's not confuse people more by exposing these here.

src/index.js Outdated
}
__MOBX_DEVTOOLS_GLOBAL_HOOK__.injectMobxReact(mobxReact, mobx)
}
export { Provider } from "./Provider"
Copy link
Contributor

@danielkcz danielkcz May 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here we should probably export the Context object as well so people can make a custom hook for it. It's to support an easier migration instead of expecting from them to rewrite to new Context in a big swoop.

Suggested change
export { Provider } from "./Provider"
export { Provider, MobXProviderContext } from "./Provider"

README.md Outdated

`mobx-react@6` and higher are no longer compatible with the mobx-react-devtools.
That is, the MobX react devtools will no longer show render timings or dependency trees of the component.
The reason is that the standard React devtools are no also capable of highlighting re-rendering components.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

devtools are no also capable - what did you mean here? Looks like a typo.

Copy link
Contributor

@danielkcz danielkcz May 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@macbem Use the suggestions next time please

Suggested change
The reason is that the standard React devtools are no also capable of highlighting re-rendering components.
The reason is that the standard React devtools are also capable of highlighting re-rendering components.

@mweststrate mweststrate merged commit d5c832f into master May 29, 2019
@danielkcz danielkcz deleted the v6 branch June 10, 2019 11:38
@vkrol vkrol mentioned this pull request Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants