Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic rendering backend targets #100

Closed
wants to merge 2 commits into from

Conversation

bodhi
Copy link

@bodhi bodhi commented Feb 12, 2017

As an experiment to use Pux with react-blessed and React Native, I made a few changes to make Pux more general: forall a. target a instead of Html a. This allowed me to reuse most of Pux to implement Screen a and Native a as backends using the Pux architecture.

This PR is mostly a proof-of-concept, and I'd be grateful for any feedback. I don't feel that it has to be merged in it's current state. Some parts are a bit odd as I tried to to maintain backwards-compatibility in Pux, while providing the generic functionality in Pux.Base. I haven't included the RN and Blessed backends, I'll hopefully put them online in a different repository shortly, but the code is a bit of a mess (doesn't everyone say that? 😹).

This:

1. Allows other renderers to be named at the type-level, instead of
   disguising themselves as `Html a`.

2. Enables the generic functions to operate with alternative
   renderers, ie instead of `Html a`, the type is `forall
   target. target a`.
Pux requires a custom `render` function since it special-cases string
parameters. To maintain backwards compatibility:

1. Move `render` to `Pux.Base`

2. Parameterise `start` (as `start'` to allow a custom `render`
   function

3. Refactor `Pux.render` as a wrapper around `Pux.Base.render` that
   maintains the existing behaviour
@chexxor
Copy link
Contributor

chexxor commented Feb 12, 2017

The diff makes it hard to read and the meat is in other repos you will make later. I don't see any reference to Native or Screen here. The important addition is wrapRender, then, to support those? Then you moved the rest to the Base.* files?

bodhi added a commit to bodhi/purescript-pux that referenced this pull request Feb 12, 2017
To make alexmingoia#100
clearer, inline the changes back into `Pux` to make for a
simpler/clearer diff.
bodhi added a commit to bodhi/purescript-pux that referenced this pull request Feb 12, 2017
To make alexmingoia#100
clearer, inline the changes back into `Pux` to make for a
simpler/clearer diff.
bodhi added a commit to bodhi/purescript-pux that referenced this pull request Feb 12, 2017
To make alexmingoia#100
clearer, inline the changes back into `Pux` to make for a
simpler/clearer diff.
bodhi added a commit to bodhi/purescript-pux that referenced this pull request Feb 12, 2017
To make alexmingoia#100
clearer, inline the changes back into `Pux` to make for a
simpler/clearer diff.
@bodhi
Copy link
Author

bodhi commented Feb 12, 2017

The diff makes it hard to read

Ah, right. I didn't think of that. Because they are copies plus small changes, it's not really obvious what the actual change is. The core of the change is really just making the exported types more generic. Html a doesn't need to be fixed, it can be inferred by:

  1. Using elements from Pux.Html.Element
  2. Passing the view signal to renderToDOM

I've made a throwaway commit that pulled the changes back into Pux, the comparison should more clearly demonstrate what I've done.

The modifications to start/start' are only necessary because render implicitly wraps string views in a <div>. If it didn't do that, I think it could be generalised to

foreign import render :: forall a eff target. Fn3 (a -> Eff eff Unit) (a -> a) (target a) (target a)

I don't see any reference to Native or Screen here.

Right, I didn't want to include those as they would then require a dependency on React Native, or react-blessed.

Here's a gist that shows (rather, "contains") a cobbled-together version of the React Native backend: https://gist.github.com/bodhi/a1327cda84cec1ac37978b5189b22f48

@Qata
Copy link

Qata commented Feb 13, 2017

@bodhi I'm very interested in getting this working with RN as well. Considering how this project has been idle for the past ~3 months, I'd suggest hosting your own version while you work on the changes needed. I'd be happy to collaborate with you.

@bodhi
Copy link
Author

bodhi commented Feb 14, 2017

@Qata it's actually possible to use RN with Pux in the current state, you just need to:

  1. Write an equivalent to renderToDOM.
  2. Pretend that Html a is the correct type 😸.
  3. Write some wrappers for React Native elements like View, etc.
  4. Be careful with Pux.Html.*: some functions like attr are useful, but have unexpected special cases (e.g. for a style attribute).

That said, if you want to make the RN code from my gist above into a real package, that would be cool.

@alexmingoia
Copy link
Owner

alexmingoia commented Feb 14, 2017

FYI the next release of Pux will be a major overhaul, and includes generic renderers (shipping with React renderer) and with purescript-smolder replacing Pux's current virtual-dom types. I haven't had much time to talk about it. Looking to release that soon, and get back to actively maintaining this library.

@bodhi
Copy link
Author

bodhi commented Feb 14, 2017

@alexmingoia sounds interesting, have you started working on it? Would be nice to see it progress ;)

@alexmingoia
Copy link
Owner

I've released Pux 8 which uses purescript-smolder for markup and includes a React renderer, which you can use as a reference implementation for other virtual DOM backends. That should allow developers to implement any kind of custom rendering or caching they need.

See the updated guide for more details, and https://pursuit.purescript.org/packages/purescript-pux/8.0.0/docs/Pux.Renderer.React

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.

None yet

4 participants