react-querybuilder/react-querybuilder

query rules update by setQuery does not update/match the public query rules

kellywang1234 opened this issue · 13 comments

Hi thanks for contributing on this awesome library!

I have an issue that bothers me a lot. I noticed that the local state query does not match the public query rules. How should we reset the public query rules?
The details of the issue is: I want to reset query to initialQuery when the number of rules is 3. The reset works but when i click the "Add Rule" button again, instead of showing one rule, 3 old rules displays. The old rules are still in ;"public" rule which is_q.rules, and setQuery won't update this public rule. Do you know how to remove the old rule in order to match the local statey query updated by setQuery()?

This is the example: when adding 3 rules, the rule reset to 0. Then click add rule again, it should display only one rule.
https://codesandbox.io/p/devbox/determined-ben-xvc8nl?file=%2Fsrc%2FApp.tsx%3A34%2C29

Version: version 7 alpha

Confirmed this is an issue. Probably to do with the new Redux internals. I'll try to fix it soon.

This is a strange use case, but the simplest way to do it in v7 would be to use the getQuery and dispatchQuery functions from props.schema in a custom addRuleAction component (and addGroupAction if that's appropriate).

Here is how I would do it:

https://codesandbox.io/p/devbox/little-butterfly-x6fgnp?file=%2Fsrc%2FApp.tsx

import { useState } from 'react';
import type {
  ActionProps,
  Field,
  RuleGroupType,
} from 'react-querybuilder';
import { ActionElement, QueryBuilder, formatQuery } from 'react-querybuilder';
import './styles.scss';

const fields: Field[] = [
  { name: 'firstName', label: 'First Name' },
  { name: 'lastName', label: 'Last Name' },
];

const initialQuery: RuleGroupType = {
  combinator: 'and',
  rules: [],
};

const AddRule = (props: ActionProps) => {
  const query = props.schema.getQuery();
  if ((query?.rules.length ?? 0) >= 2) {
    return (
      <ActionElement
        {...props}
        handleOnClick={() => props.schema.dispatchQuery(initialQuery)}
      />
    );
  }
  return <ActionElement {...props} />;
};

export const App = () => {
  const [query, setQuery] = useState(initialQuery);

  return (
    <div>
      <QueryBuilder
        fields={fields}
        query={query}
        onQueryChange={setQuery}
        controlElements={{
          addRuleAction: AddRule,
        }}
      />

      <h4>Query</h4>
      <pre>
        <code>{formatQuery(query, 'json')}</code>
      </pre>
    </div>
  );
};

Thank you for making this happens, it works in the sandbox! we may close this ticket. The issue still exists because your solution is to reset the query by using props.schema.dispatchQuery(initialQuery) where props is part of the ActionProps. For my project, I still need to reset the query without the help of ActionProps. In conclusion, the ideal outcome is in useEffect whenver label(dropdown options) changes, setQuery(initialQuery) should do the same functionality as props.schema.dispatchQuery(initialQuery). So that next time when click +Rule button, the query is clean and reset to default. I added a video below to explain the issue in my project. Would it be fixed in the next version release?
issue.zip
Screenshot 2024-01-07 at 12 37 33 AM

So, I do agree it's an issue that should probably be addressed. But I'm having trouble seeing a reason it would ever be a blocker. I really think useEffect is the wrong choice in your case, and avoiding it seems to avoid the issue.

I'll leave this issue open for a little while so it's visible and you can respond, but I don't plan to address it soon unless I can be convinced otherwise.

So, I do agree it's an issue that should probably be addressed. But I'm having trouble seeing a reason it would ever be a blocker. I really think useEffect is the wrong choice in your case, and avoiding it seems to avoid the issue.

I'll leave this issue open for a little while so it's visible and you can respond, but I don't plan to address it soon unless I can be convinced otherwise.

https://codesandbox.io/p/devbox/dreamy-swartz-88fcxv?file=%2Fsrc%2FApp.tsx%3A33%2C10 Here comes the issue if setQuery() is not really cleaning up: When adding some rules then hit refresh, then hit add Rule button, old rules still display. We do not want to see old rules. The issue is not really about useEffect, it is about setQuery()

Ok, now I see how it could be an issue even without useEffect. That's definitely a blocker for v7. I'll try to fix it and release another v7 alpha soon.

@kellywang1234 I think I've got this resolved in #638. You can test the new code in this sandbox, which has the same app code as the last one you linked to but with the (hopefully) fixed version of RQB.

EDIT: I made some updates; new sandbox is here.

@kellywang1234 I think I've got this resolved in #638. You can test the new code in this sandbox, which has the same app code as the last one you linked to but with the (hopefully) fixed version of RQB.

EDIT: I made some updates; new sandbox is here.

Thank you for fixing this issue! You are the most responsible community contributor i have seen. would you mind me asking approximately when will the fixed version release?

Thank you for fixing this issue! You are the most responsible community contributor i have seen. would you mind me asking approximately when will the fixed version release?

Thanks! That's very kind of you to say.

The latest v7 alpha includes this fix: v7.0.0-alpha.6.

Thank you for fixing this issue! You are the most responsible community contributor i have seen. would you mind me asking approximately when will the fixed version release?

Thanks! That's very kind of you to say.

The latest v7 alpha includes this fix: v7.0.0-alpha.6.

when i upgrade from v7 alpha.2 to v7 alpha.6, it throw me error saying my node version 14.21.1 is not compatible, it requires >=16 . Did you upgrade the node version in alpha.6? alpha.2 is still compatible with node 14.21, but not alpha.6 anymore.

Weird. I don't recall making any changes that would have affected the minimum Node version.

That said, I can't say we'll ever support a Node version that's hit end-of-life. Node 14 was EOL'd over two years ago, and even Node 16 was EOL'd over a year ago. I would highly recommend upgrading to at least 18 (EOL in ~1 year) if not 20 (EOL in ~2 years).

Weird. I don't recall making any changes that would have affected the minimum Node version.

That said, I can't say we'll ever support a Node version that's hit end-of-life. Node 14 was EOL'd over two years ago, and even Node 16 was EOL'd over a year ago. I would highly recommend upgrading to at least 18 (EOL in ~1 year) if not 20 (EOL in ~2 years).
from The error itself, it seems numberic-quantity requires newer version of node. If you didn't upgrade the node version, maybe you upgraded the numberic-quantity?

Screenshot 2024-02-01 at 8 07 55 PM

Oh, right. Now I remember. I added the numeric-quantity package which has a "engines" property in its package.json.

My recommendation re: Node <18 stands though.