Closed cristiano-belloni closed 5 years ago
@gabrielmontagne Let's not merge this until we run npm i
and update package-lock.json
I like this! Tho we need to make sure of the packaging. On a CRA2 + electron app I'm trying this out I'm getting compilation problems,
./node_modules/@zambezi/caballo-vivo/src/state-context.js
SyntaxError: C:\Users\gabriel\Documents\los-fuegos\lupe-sentinela-electron\node_modules\@zambezi\caballo-vivo\src\state-context.js: Unexpected token (8:6)
6 | return function contextView(state) {
7 | return (
> 8 | <StateContext.Provider value={state}>
| ^
9 | {toView(state)}
10 | </StateContext.Provider>
11 | )
... tho it's very likely it's on my end. I need to dig deeper.
I'm running it directly from GH,
+++ b/package.json
@@ -67,6 +67,7 @@
"deb": {}
},
"dependencies": {
+ "@zambezi/caballo-vivo": "https://github.com/zambezi/caballo-vivo#pluggable-state-sketch",
"d3-format": "^1.2.2",
"d3-scale": "^2.0.0",
"eases": "^1.0.8",
We decided to not transpile/package/bundle it, so the problem is probably on the user's end? Are you using CRA2, with ES6 support?
Will try on a fresh project. I migrated with who knows how much success. I agree, no package is the way. Let me see.
On Thu, 14 Feb 2019, 16:09 Cristiano Belloni, notifications@github.com wrote:
We decided to not transpile/package/bundle it, so the problem is probably on the user's end? Are you using CRA2, with ES6 support?
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/zambezi/caballo-vivo/pull/15#issuecomment-463661740, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF2qUe6d5FC2niNWe-a0nVZ0Eum7-ivks5vNXwMgaJpZM4a2Ldv .
We just discovered that CRA2 doesn't allow importing JSX from node_modules
(for good reasons, it seems).
I can make a non-JSX version of this, because transpiling is a huge pain and this component is nice and small. @gabrielmontagne would you be OK with that?
I think it will be something really easy and even quite readable like:
React.createElement(StateContext.Provider, {
value: state
}, toView(state))
I tried it at home and it works, we can probably merge this.
Sounds great. Let's bring this in.
@cristiano-belloni , because of this new state sharing magic, we now need react as a proper dependency (in testing, I'm finding out now).
It'll come as part of the stow
operator PR. Let me know if you think it shouldn't be an explicit dependency.
It seems we are already depending on React >= 16.8.0, as in the package. Or maybe I didn't understand?
Yeah, it was there. Strange, man, never mind.
On Sat, 23 Mar 2019 at 14:46, Cristiano Belloni notifications@github.com wrote:
It seems we are already depending on React >= 16.8.0, as in the package https://github.com/zambezi/caballo-vivo/blob/master/package.json. Or maybe I didn't understand?
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/zambezi/caballo-vivo/pull/15#issuecomment-475875687, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF2qSw6tEKqRNogEp2bOG-jmp_r4XRKks5vZj47gaJpZM4a2Ldv .
This is to start discussion on a possible "pluggable state" enhancement.
How it works:
state-context.js
exports a factory, calledcreateStateContext
, which can be used to wrap the (userland)toView
function with a React context which tracks the statestate-context.js
also exports a hook, calleduseStateContext
, which can be used by any descendant component of the view to immediately access the state without passing it down the component chain.Obviously, this can be expanded to support more than one context, or passing down just a part of the state (but is it worth? we're passing references anyway)