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

Error when loading in JSON saving and removing last rule in group #190

Closed
meg8751 opened this issue Apr 6, 2020 · 5 comments · Fixed by #285
Closed

Error when loading in JSON saving and removing last rule in group #190

meg8751 opened this issue Apr 6, 2020 · 5 comments · Fixed by #285

Comments

@meg8751
Copy link

meg8751 commented Apr 6, 2020

Describe the bug
When I save and load value from the json version saved in state, instead of the Immutable Tree version, I am seeing an error when a group is left empty. Most of the time this works fine and there are no problems, but if a group is left empty either by dragging or deleting an error is thrown.

Apologies if I am missing something obvious, or if there is some reason this should never work, but saw the same problem both in my own code and when I modified and ran demo.tsx.

To Reproduce

  1. Update demo.tsx to look like version included in Additional Context below.
  2. Run npm run examples from awesome query builder repo.
  3. Delete or drag out the last rule in a group.
    2020-04-06 13 12 15
    2020-04-06 13 18 56

Expected behavior
2020-04-06 13 22 07

Additional context
Here is what my modified version of demo.tsx looks like:

import React, {Component} from 'react';
import {
    Query, Builder, BasicConfig, Utils,
    //types:
    ImmutableTree, Config, BuilderProps, JsonTree, JsonLogicTree, JsonGroup
} from 'react-awesome-query-builder';
import throttle from 'lodash/throttle';
import loadedConfig from './config';
import loadedInitValue from './init_value';
import loadedInitLogic from './init_logic';

const stringify = JSON.stringify;
const {queryBuilderFormat, jsonLogicFormat, queryString, mongodbFormat, sqlFormat, getTree, checkTree, loadTree, uuid, loadFromJsonLogic} = Utils;
const preStyle = { backgroundColor: 'darkgrey', margin: '10px', padding: '10px' };
const preErrorStyle = { backgroundColor: 'lightpink', margin: '10px', padding: '10px' };

const emptyInitValue: JsonTree = {id: uuid(), type: "group"};
let initValue: JsonTree = loadedInitValue && Object.keys(loadedInitValue).length > 0 ? loadedInitValue as JsonTree : emptyInitValue;
let initLogic: JsonLogicTree = loadedInitLogic && Object.keys(loadedInitLogic).length > 0 ? loadedInitLogic as JsonLogicTree : undefined;
let initTree;
initTree = checkTree(loadTree(initValue), loadedConfig);
//initTree = checkTree(loadFromJsonLogic(initLogic, loadedConfig), loadedConfig); // <- this will work same

const updateEvent = new CustomEvent('update', { detail: {
        config: loadedConfig,
        _initTree: initTree,
        _initValue: initValue,
    } });
window.dispatchEvent(updateEvent);


interface DemoQueryBuilderState {
    jsonTree: JsonGroup;
    config: Config;
}

export default class DemoQueryBuilder extends Component<{}, DemoQueryBuilderState> {
    private jsonTree: JsonGroup;
    private config: Config;

    state = {
        jsonTree: initValue,
        config: loadedConfig
    };

    render = () => (
        <div>
            <Query
                {...this.state.config}
                value={loadTree(this.state.jsonTree)}
                onChange={this.onChange}
                renderBuilder={this.renderBuilder}
            />

            <button onClick={this.resetValue}>reset</button>
        </div>
    )

    resetValue = () => {
        this.setState({
            jsonTree: initValue,
        });
    };

    renderBuilder = (props: BuilderProps) => (
        <div className="query-builder-container" style={{padding: '10px'}}>
            <div className="query-builder qb-lite">
                <Builder {...props} />
            </div>
        </div>
    )

    onChange = (immutableTree: ImmutableTree, config: Config) => {
        this.jsonTree = getTree(immutableTree);
        this.config = config;
        this.updateResult();
    }

   updateResult = throttle(() => {
        this.setState({jsonTree: this.jsonTree, config: this.config});
    }, 100)

}
@ukrbublik ukrbublik added wontfix and removed later labels Jun 8, 2020
@qurben
Copy link

qurben commented Sep 4, 2020

Is this issue marked won't fix because JsonTree is used as storage in the state instead of ImmutableTree?

For me it seemed reasonable to use JsonTree in my React state because it is easier to interact with, but it looks like loadTree(getTree(immutableTree)) is does not always return a usable tree.

@ukrbublik
Copy link
Owner

Yes.
Internal state of query builder is in Immutable format, for historical reasons.

@meg8751
Copy link
Author

meg8751 commented Sep 4, 2020

@qurben FYI I ended up adding a work around in my own code to change the query builder's behavior so that a group disappears when the last item is deleted or dragged out instead of maintaining the empty group. I needed to save my state in Json Rules Engine (https://github.com/CacheControl/json-rules-engine) format and it was way easier to convert to Rules Engine format from the json tree.

Ignoring my Json Rules Engine conversion, I am recursively stripping out empty groups after getTree() before saving to state, so that when it is loaded from state into immutable format with loadTree() the empty group does not exist, no error is thrown, and the tree is usable.

To save:
Immutable -> JsonTree -> Strip empty groups from JsonTree -> state

To load:
State (json format without empty groups) -> Immutable

Not an ideal solution for several reasons but it worked for my use case.

@qurben
Copy link

qurben commented Sep 4, 2020

I worked around this by just using ImmutableTree as the state. Which requires some extra logic when other parts use the tree, but it works.

@ukrbublik ukrbublik linked a pull request Sep 9, 2020 that will close this issue
@ukrbublik
Copy link
Owner

Fix: this.jsonTree = getTree(immutableTree, false);
I'll update TS def for 2nd param of getTree in next version

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 a pull request may close this issue.

3 participants