Performance issue

[hyanmandian]

Hello :D I don't know why, but I have a performance issue with version 2.4.0, I think that the changes made from 2.3.1 to 2.4.2 make this happen. I don't have time to help you to found it right now, but I would like to warn.

I suppose that the lifecycles refactoring that you did make this issue (maybe why-did-you-update lib can help you to debug)

Thanks!

[ryami333]

Hi @hyanmandian, could you describe what you mean by a 'performance issue'? Have you actually verified that memory usage is increasing? Extra renders are happening? Would you please be able to send us a Codesandbox to demonstrate how you're using the library?

[hyanmandian]

Sure, good idea :D

Version: 2.3.1 -> https://codesandbox.io/s/5z55m8wk3k
Version: 2.4.2 -> https://codesandbox.io/s/3r30opwlv1

[hyanmandian]

If you use "why-did-you-update" library, you will see a lot of logs about Provider and Consumer components

[ryami333]

OK, thanks for the details. Will have a bit of dig!

[hyanmandian]

Sorry, miss click...

So, I think that this issue happens in this file -> https://github.com/springload/react-accessible-accordion/blob/master/src/AccordionItem/accordion-item-wrapper.js

Maybe because you create a Provider and Subscribe for each item.

[hyanmandian]

I have an idea to solve that. I will send you a PR with this idea, just sec.

[hyanmandian]

I tried in here, but I think that I need to study how use TS first (I don't have any exp with TS)... But basically the idea is move the accordion-item-wrapper.js Provider and Subscribe components to accordion.js and pass those props to childrens using React.cloneElement.

Something like that:

// accordion.js
render() {
        const { accordion, children, ...rest } = this.props;

        return (
            <Provider inject={[this.itemContainer]}>
                <Subscribe to={[AccordionContainer, ItemContainer]}>
                    {(accordionStore, itemStore) => {
                        const { uuid } = itemStore.state;
                        return (
                            <div role={accordion ? 'tablist' : null} {...rest}>
                                {React.Children.map(children, (item) => React.cloneElement(item, {
                                    accordionStore,
                                    itemStore,
                                }))}
                            </div>
                        );
                    }}
                </Subscribe>
            </Provider>
        );
    }

If I have time, I will try to help you with it.

[ryami333]

@hyanmandian, our apologies for the delay - but I think I've resolved this. I've published a pre-release 2.4.4-beta.1 to check this out with your codesandbox and everything looks better to me now. Could you please check too? https://codesandbox.io/s/jz80xw6o29

[hyanmandian]

@ryami333 Sorry for the delay :(I tested the last version 2.4.4 and the problem is the same -> https://codesandbox.io/s/z60nz3nnv4

[PavelZhuravlev]

any updates here?
same issue with a latest version of the lib
demo - https://stackblitz.com/edit/github-2gqn2z?file=pages/index.js