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

How one can wrap ALL external components in observer without recompilation? #1107

Closed
stgolem opened this issue Jul 21, 2017 · 7 comments
Closed

Comments

@stgolem
Copy link

stgolem commented Jul 21, 2017

We all have to read #101 here, but whats next?

Can anyone please provide me an example of using mobx with big 3-rd party components library like https://github.com/ant-design/ant-design?
We are stuck with Table and all its nested components. Here is basic example:

@observer
export class ProcessList extends React.Component<ProcessListProps, ProcessListState> {
    constructor(props: ProcessListProps) {
        super(props);
        this.state = {
            ProcessList: (this.props[processlist] as ProcessListService).getProcessTree,
            isLoading: (this.props[processlist] as ProcessListService).isLoading,
            modelVisible: false
        };
    }

    componentWillReceiveProps(nextProps: ProcessListProps) {
        this.setState({
            ProcessList: (this.props[processlist] as ProcessListService).getProcessTree,
            isLoading: (this.props[processlist] as ProcessListService).isLoading
        });
        return true;
    }
    
    editProcess = (record) => {
        this.setState({modelDialog: record, modelVisible: true});
    }

    render() {
        const { ProcessList } = this.state;
        
        return (
            <div style={{ textAlign: 'center' }}>
                <Table rowKey={(record) => record.Uid} dataSource={ProcessList}>
                    <Column key="Name" title="Name" dataIndex="Name" />
                    <Column key="edit" title="edit" render={(text, record) => (
                    <span>
                    <Button onClick={() => this.editProcess(record)}>Edit</Button>
                    </span>
                )} /> 
                </Table>
                <ModalComponent model={this.state.modelDialog} visible={this.state.modelVisible}/>
            </div>
        );
    }
}

As you can see here we have Table and Column that is bound to dataIndex by property name. To make it work we have to recompile whole ant-design project, that have references on more internal https://github.com/react-component components (we have to mark them ALL as observer also).
That is... a lot of unpaid work we have to to ((

How one can wrap ALL external components in observer without recompilation?
Isn't it confusing and misleading with whole MobX concept?
Can we remove this limitation from our mobx sources?

@mweststrate
Copy link
Member

mweststrate commented Jul 25, 2017

The basic idea is not to make external components observer, but instead not pass observables to them. Instead, convert any observable data to non-observable data (in the format the 3th party components expect), in the closest observer parent component. (Make sure to do this as part of rendering or with a computed property).

@stgolem
Copy link
Author

stgolem commented Jul 25, 2017

This is the default pattern wherever possible. But this is about rendering Table. It can contain decent amount of data. I really do not want to rerender whole table when single data row changes.
I was thinking about HOC that can override rendered elements and make them observer recursive.

@urugator
Copy link
Collaborator

urugator commented Jul 26, 2017

Firstly you can't make an element observer, but a component. You can't "hook into" render functions and change which components are being used to render elements.
Secondly converting the components to observers may not suffice, the observable data has to be accessed in observable way - dereferencing observables lately in the component which should be re-rendered.

If you can provide a custom render function, you may use Observer component:

<Column render={(text, record) => <Observer>{() => { /* dereference observable object here, like record.text */ }}</Observer>}>

EDIT: Btw it's the library, which doesn't have means to perform a "partial" component update. Or is there any non-MobX solution which allows that?

@stgolem
Copy link
Author

stgolem commented Jul 27, 2017

I've tried Observer component solution, and it does not work if i do like this:

@observer
export class ObservableTableColumn extends React.Component<{ key : string; title : string; dataIndex : string; }> {
    render() {
        return (
            <Table.Column key={this.props.key} title={this.props.title} render={(text, record) => (
                <Observer>{() => (
                    <span>{record[this.props.dataIndex]}</span>
                )}
                </Observer>
            )} />
        );
    }
}

The idea here is that i need generic column for showing some property from objects.

@urugator
Copy link
Collaborator

Well record must be an observable, I don't know what the record is, I am not familiar with the library... if it is and it's changed the <span> will re-render, thats it...

@stgolem
Copy link
Author

stgolem commented Jul 27, 2017

Ok, here's more code to clarify

@observer
export class ObservableTableColumn extends React.Component<{ key: string; title: string; dataIndex: string; }> {
    render() {
        return (
            <ProcessColumn key={this.props.key} title={this.props.title} render={(text, record) => (
                <Observer>{() => (
                    <span>{record[this.props.dataIndex]}</span>
                )}
                </Observer>
            )} />
        );
    }
}

@observer
export class RenderProcessTable extends React.Component<RenderProcessTableProps> {

    shouldComponentUpdate(nextProps: RenderProcessTableProps) {
        if (this.props.dataSource) {
            return !nextProps.loading && nextProps.dataSource.length != this.props.dataSource.length;
        }
        return true;
    }

    render() {

        const processTree = this.props.loading ? [] : this.props.dataSource.slice(0);

        return (
            <ProcessTable rowKey={(record) => "process-row@" + record.Id} dataSource={processTree} loading={this.props.loading}>
                <ProcessColumn key="name" title="Name" render={(text, record) => (
                    <Observer>{() => (
                        <span key={"name-cell-span@" + record.Id}>{record.Name}</span>
                    )}
                    </Observer>
                )} />
                <ObservableTableColumn key="name2" dataIndex="Name" title="Name-2" />
                <ProcessColumn key="edit" title="Edit" render={(text, record) => (
                    <Observer>{() => (
                        <span key={"edit-cell-span@" + record.Id}>
                            <Button key={"edit-cell-button@" + record.Id} onClick={() => this.props.editClick(record)}>Edit</Button>
                        </span>
                    )}
                    </Observer>
                )} />
            </ProcessTable>
        );
    }

};

Here we have 2 components. In RenderProcessTable we use 2 columns to display this.props.dataSource (that is observable already at the top component).
First one is to use ProcessColumn directly with combination of Observer component.
Second one is with more generic ObservableTableColumn which should read its data from row record.

The trick here is that the first column is now bound to observable item but the generic one it not and it is not rerender after editing record.Name.

If you can point to solution here, i would really appreciate.

@mweststrate
Copy link
Member

@stgolem either use the non generic one (or express the generic one with in terms of the non generic one, with help of @observer / Observer. Or don't pass in observable data to to the processTable, but convert to plain objects first (which is expensive for large amounts of data).

I would recommend the first approach. We can't do much more for you. If the data is observable, but the data consumer isn't based on MobX, there is simply no way we can still super magically give all the benefits of mobx to that non-mobx aware consumer.

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

No branches or pull requests

3 participants