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

useParseQuery(query) returns previous results when passed a new query #81

Open
dr-skot opened this issue Mar 14, 2022 · 5 comments
Open
Labels

Comments

@dr-skot
Copy link

dr-skot commented Mar 14, 2022

const { isLive, isSyncing, isLoading, results, error } = useParseQuery(query);
  1. when query changes you get
{ results: resultsOfPreviousQuery, isLoading: false, isLive: true, isSyncing: false, error: undefined }
  1. a little later you get a re-rerender with the appropriate response
{ results: undefined,  isLoading: true, isLive: true, isSyncing: false, error: undefined }
  1. and when the fetching is done
{ results: newResults, isLoading: false, isLive: true, isSyncing: false, error: undefined }

Response (2) should be what's returned at step (1). A new query shouldn't return results from a previous one.

@parse/react-ssr 0.0.1-alpha.17


Here's a page for reproducing it:

import { initializeParse } from '@parse/react-ssr';
import { useEffect, useState } from 'react';
import { useParseQuery } from '@parse/react-ssr';

initializeParse(
  process.env.NEXT_PUBLIC_PARSE_SERVER_URL!,
  process.env.NEXT_PUBLIC_PARSE_APP_ID!,
  process.env.NEXT_PUBLIC_PARSE_JS_KEY!
);

const OBJECT_CLASS = 'Showtime';

interface LogEntry {
  requestedId?: string;
  resultId?: string;
  isLoading: boolean;
  isSyncing: boolean;
  isLive: boolean;
  error?: Error;
}

export default function FalseZero() {
  const [ids, setIds] = useState<string[]>([]);
  const [requestedId, setRequestedId] = useState('');
  const [log, setLog] = useState<LogEntry[]>([]);

  const query = new Parse.Query(OBJECT_CLASS).equalTo('objectId', requestedId);
  const { isLive, isSyncing, isLoading, results, error } = useParseQuery(query);
  const resultId = results?.[0]?.id;

  // log results when they change
  useEffect(() => {
    setLog((prev) => [...prev, { requestedId, resultId, isLoading, error, isLive, isSyncing }]);
  }, [requestedId, resultId, isLoading, error, isLive, isSyncing]);


  // on mount, load valid ids
  useEffect(() => {
    new Parse.Query(OBJECT_CLASS)
      .find()
      .then((results) => setIds(getIds(results)))
      .catch(console.error);
  }, []);

  // request a random id
  function fetchRandomObject() {
    if (!ids?.length) return;
    const i = Math.floor(Math.random() * ids.length);
    setRequestedId(ids[i]);
  }

  if (!ids?.length) return <>no ids yet</>;

  return (
      <>
        <button className={'btn btn-sm btn-primary'} onClick={fetchRandomObject}>
          fetch
        </button>
        <button className={'btn btn-sm btn-primary ml-2'} onClick={() => setLog([])}>
          clear log
        </button>
        <p>{resultId}</p>
        <p>---</p>
        <p>{log.length} entries</p>
        {log.map(reportLogEntry)}
      </>
  );
}

function reportLogEntry(entry: LogEntry, i: number) {
  const { isLoading, error, requestedId, resultId, isSyncing, isLive } = entry;
  const bad = !isLoading && !error && requestedId !== resultId;
  return (
    <p key={i} style={{ marginTop: '1em', color: bad ? 'red' : 'black' }}>
      requested: {requestedId || '[none]'}
      <br />
      result: {resultId || '[none]'}
      <br />
      isLoading: {isLoading ? 'true' : 'false'}
      <br />
      error: {entry.error?.message || '[none]'}
      <br />
      isLive: {isLive ? 'true' : 'false'}
      <br />
      isSyncing: {isSyncing ? 'true' : 'false'}
    </p>
  );
}

function getIds(list: { id: string }[]) {
  return list.map(({ id }) => id);
}
@dr-skot
Copy link
Author

dr-skot commented Mar 14, 2022

I posted a workaround

@davimacedo
Copy link
Member

@dr-skot thanks for reporting. Would be willed to open a PR with the fix as well?

@dr-skot
Copy link
Author

dr-skot commented Mar 18, 2022

@davimacedo

Sorry, I don't have the fix, just a workaround. I looked in the codebase to see if I could help but I'd need to spend more time than I have right now trying to grok exactly what the reducer in parse-react-base/src/useParseQuery.ts is doing. I suspect the problem is in that logic somewhere.

All my workaround does is mistrust the first response from response = useParseQuery(query) whenever query changes.

Specifically, it returns { ...response, isLoading: true, results: undefined } the first time, and response thereafter. Until query changes again.

But also this workaround isn't sufficient. There's another problem when fetching a newly created object where the 1st, 3rd, 4th, and 5th responses are wrong.

Here's what I've observed, using this kind of query

const query = new Parse.Query(OBJECT_CLASS).equalTo('objectId', requestedId);
const response = useParseQuery(query);

When requestedId is the id of an already-existing object, the sequence of response values is

existing-object

when requestedId is the id of a newly-created object, ie,

requestId = (await new Parse.Object(OBJECT_CLASS).save()).id

it's like this

new-object

Maybe this is the expected behavior? I mean 1 has to be considered a bug, I think. But in 3, 4, 5 it looks like I'm getting a result from the local cache, and then in 6 the real result from the server arrives. That's cool, but I need some way to distinguish state 4 from state 6, so I don't flash NOT FOUND at the user. And all the flags are the same in states 4 and 6.

Probably what's right is to remove states 1 and 4.

2 = loading (isLoading)
3 = cached value retrieved (!isLoading && !isLive)
5 = cached value retrieved, now syncing with server (!isLoading && isLive && isSyncing)
6 = server value retrieved (!isLoading && isLive && !isSyncing)

@dr-skot
Copy link
Author

dr-skot commented Mar 18, 2022

FYI I've updated my workaround. It's still just a wrapper around useParseQuery that corrects responses 1 and 4 (see tables in previous comment) to responses 2 and 5.

Now I'm getting what I think is the appropriate behavior...

For an object that's in the local cache,
Screen Shot 2022-03-18 at 12 45 01 PM

For an object that's not,
Screen Shot 2022-03-18 at 12 45 29 PM

(the status column is my interpretation of what the flags mean in combination)

I hope this information is helpful. Sorry it's not a PR. Maybe I'll have time to dive into the code this weekend.

@davimacedo
Copy link
Member

Thanks for the detailed information on the workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants