Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Question: Any advice if my list of dynamic components is also not static? [@redux-dynostore/react-redux] #10

Closed
MillerGregor opened this issue Jun 20, 2018 · 11 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@MillerGregor
Copy link
Contributor

For my needs, I'd like to create dynomic components from a list of identifiers that isn't static. This list would ideally be in a separate slice of the redux state.

I implemented this in my app, but am running into recursive update problems.

Here's a codesandbox with the counters example, modified to illustrate:
https://codesandbox.io/s/1r2wwjp2pl

Can anyone suggest an idea to make this work?
thanks

@mpeyper mpeyper added the question Further information is requested label Jun 20, 2018
@mpeyper
Copy link
Contributor

mpeyper commented Jun 20, 2018

Hey,

The issue was caused by calling Counter.createInstance(id) inside a render function. This creates a new higher-order component which should not be done inside a render method.

Here's a fork of your codesandbox that works, take particular note of the DynaCounter component:
https://codesandbox.io/s/l7knkmp089

@mpeyper
Copy link
Contributor

mpeyper commented Jun 20, 2018

I've seen this type of issue come up a few times where people have wanted to use a dynamic identifier. I'm wondering if there is some way we can support this?

From the top of my head, one option might be to accept an identifier as a prop that overrides the provided one, e.g.

const App = ({ counters, dispatch }) => {
  const action = (type, value) => () => dispatch({ type, value })
  return (
    <div>
      <div>
        {counters.map(id => {
          return <Counter key={id} identifier={id} />
        })}
      </div>
      <button onClick={action('ADD', 'three')}>add "three"</button>
    </div>
  )
}

This would make the createInstance functionality a bit moot though.

It would also allow for index based identifiers a bit easier, e.g. "counter1", "counter2", "counter3", etc.

@MillerGregor
Copy link
Contributor Author

Excellent - thank you.

I should have figured that out. It's easy (for me) to forget that the whole body of a functional component is the render function.

A component like you suggest might be very useful. It's the declarative option to the imperative createInstance().

@MillerGregor
Copy link
Contributor Author

@mpeyper,

If you'd rather not add a component to the library, I'd be happy to add this example via PR. Let me know.

@MillerGregor MillerGregor reopened this Jun 20, 2018
@mpeyper
Copy link
Contributor

mpeyper commented Jun 20, 2018

I think the example is worth it, regardless of whether we implement the feature.

My only thought is that the example got a bit wonky if you pressed the "add three" button multiple times. If it could "add three", then "add four", etc. that would be better.

@jpeyper what do you think about an identifier prop?

@mpeyper mpeyper added the enhancement New feature or request label Jun 20, 2018
@mpeyper
Copy link
Contributor

mpeyper commented Jun 20, 2018

@Gregor1971 it would be fun if you could have a counter control the number of counters to display 😆

@jpeyper
Copy link
Collaborator

jpeyper commented Jun 20, 2018

I was wondering about this the other day when answering the other issues.

It actually makes more sense to me than the current approach.

@mpeyper
Copy link
Contributor

mpeyper commented Jun 20, 2018

Do we have both or remove createInstance?

This has me thinking about #9... I think we could provide a withExtraState like functionality by passing the props into the enhancers and letting then choose either their provided options or and override from props.

e.g. change this line to something like this.EnhancedComponent = dynamicEnhancer(context.store)(Component, props)

@MillerGregor
Copy link
Contributor Author

MillerGregor commented Jun 20, 2018

@mpeyper

it would be fun if you could have a counter control the number of counters to display 😆

Wouldn't be difficult... but I like to keep example code as simple as possible. 😄

My only thought is that the example got a bit wonky if you pressed the "add three" button multiple times. If it could "add three", then "add four", etc. that would be better.

yeah, that was quick and dirty. the PR is slightly less dirty... I added a text input for the identifier.

@mpeyper
Copy link
Contributor

mpeyper commented Jun 21, 2018

Cool. I'll take a look a bit later.

@mpeyper mpeyper mentioned this issue Jun 26, 2018
@mpeyper mpeyper changed the title [@redux-dynostore/react-redux] Question: Any advice if my list of dynamic components is also not static? Question: Any advice if my list of dynamic components is also not static? [@redux-dynostore/react-redux] Jun 30, 2018
@mpeyper mpeyper mentioned this issue Jan 21, 2020
@mpeyper
Copy link
Contributor

mpeyper commented Jan 27, 2021

Closing. Please see #484 for details.

@mpeyper mpeyper closed this as completed Jan 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants