purescript-react/purescript-react-basic

Proposal: separate react-dom

dwhitney opened this issue · 17 comments

Hey all, I really like this library, but I can't use it because I am concurrently building a react-native and a react-dom app. If you have any reference to react-dom in a react-native app, your app will immediately crash. How open would you be to making a purescript-react-basic-dom library that separated out all of the react-dom stuff? I'd be happy to make a pass at it for you, but I thought I'd ask before putting any work into it.

It’s already structured to make that possible but I haven’t tested it. The DOM submodules are self contained. It may be possible to write React.Basic.Native modules in this library which don’t interact with the DOM ones, if the web and native tools avoid bundling those modules in the wrong projects (which should be fine, both webpack and purs can do this). If not, separate repos works too, we just haven’t had a reason to do it yet.

Seems like the only problem is https://github.com/lumihq/purescript-react-basic/blob/4c2c2634950530200fd3d246b0dcc9b3053f060c/src/React/Basic.purs#L38

It sounds like a pain to have separate libraries for things that really should be developed together, but as long as that problem point is addressed, there shouldn't be any problems I'd think (e.g. https://github.com/justinwoo/purescript-milkis)

Ah yes, that capture definition will need to move

I see - react-dom is not in package.json, so it may work. I'll give it a try and report back.

w00t!
react-basic-native-480p

I'll experiment with what @justinwoo mentioned above -- I'm not a react expert, so I am not sure exactly what to test

hey everyone, slow-but-sure I've made progress on this: https://github.com/dwhitney/purescript-react-native-basic

I've got it to the point where I'm parsing the Typescript type declaration file for react native so that I can codegen the props types in much the same way that it's done in this project. Everything is in place for this now, but I need some help on the type-fu front

As you know, In react-basic you can pass a record of props that contains a subset of acceptable fields via that Union type class. I'd like to do that same, but in react-native the props records are not flat, meaning they can contain nested records. Do any of you have any idea how I can make the nested records "optional" in the same way they are in react-basic?

The closest attempt I've got is this: https://gist.github.com/dwhitney/8bc8bcc442135db71a40df9fe0990049

but if you leave out the "c" property (nested record), it won't compile, like this example: https://gist.github.com/dwhitney/20b567549c39c3ed8b9ce150bab7570d

any ideas on what I can do to make this work?

Thanks!
cc @joelmccracken

That's great!

I think you could do it similarly to the way CSS works. What I mean is that right now css takes any record, but I'd like to do the same Union thing to actually validate the properties. To do so, the prop type would still be CSS, but the css function would change to use that Union pattern. You'd need to create an opaque type for each of these nested shapes and write a function from the record to that type.

yeah don't know why I didn't think of that. thanks for pointing it out! I'll keep you posted as I make more progress

just an update - getting closer! I've got codegen mostly working, but still a few edge cases left to work out: https://github.com/dwhitney/purescript-react-native-basic/blob/master/src/Props.purs

That's awesome, I'm excited to try this!

screen shot 2019-02-07 at 11 45 54 am

It's working! (I changed the name of the repo to more closely match the package names)

https://github.com/dwhitney/purescript-react-basic-native

There are still three or four props missing that were giving me grief while code-gening them, and the codegen is a bit of a mess ATM, so I need to do some cleaning, but by and large I think this should work. I haven't done much testing beyond this "Hello, World!" example, but theoretically it's good.

One bit of trouble I ran into right off the bat is I can't use React.Basic.DOM (text) because Dom.js requires react-dom, which causes the React Native app to immediately crash, so I've just been using unsafeCoerce to make strings into JSX, but that's a bit ugly. um.... suggestions?

Nice! I was just thinking about playing with this last night. I'd just expose the same text function from React.Basic.Native. Another option is moving the existing one up into React.Basic but I wasn't 100% sure it would be allowed in all backends and it's not much code to duplicate.

(btw, the current version of react-basic resolves some of the issues with functions like capture mentioned above, so it might be ok to close this now)

Do you mind if I leave this open for another week or two to ask questions? I think I'll have several, and this seems like as good a place any any to ask them? Of if you'd rather close this, I can ask in some other way.

Also there is a Text component, so I've already got a text function: https://github.com/dwhitney/purescript-react-basic-native/blob/master/src/React/Basic/Native/Generated.purs#L2532

I could maybe call it string or something.

It'd be better to make an issue on your project. I'm following it.

I'm going to close this and make an issue to consider moving text (it could be reexported from DOM to make it a minor change)