Bug: Booklist page is broken

[gawlster]

Describe the Bug/Issue The booklist page ends up in an infinite loop. There are two cases here:

1. When you have one or more courses selected for a given term, it will try to fetch the textbook for each of your courses indefinitely
2. When you have zero courses selected, there is an infinite uncaught exception, which doesn't reproduce locally (at least on the main branch).

These are my thoughts after a quick investigation (take these with a grain of salt as I have never contributed to this repo):

---

Something is wrong with useTextbooks. It seems to never respond successfully. I've only seen 404s. Probably check this out and run some tests to make sure data is being returned properly.

---

There is a side-effect somewhere caused by the useTextbooks hook which forces a full re-render, and I think it has something to do with the Page component. Since the fetch for textbook data happens on mount, this is what is causing the infinite fetch. This is how I tested my hypothesis:

This is basically the BooklistContainer component which has the infinite fetch issue.

export function BooklistContainer(): JSX.Element | null {
  const [term] = useTerm();
  const textbooks = useTextbooks(term);
  useEffect(() => {
    logEvent('textbooks_view', {term});
  }, [term]);

  return (
    <Page title="">
      {/*
          Anything can go in here, even without anything you still get the issue, hence why I think it
          is the Page component causing problems
      */}
    </Page>
  );
}

Replacing it with the following obviously just shows a blank screen, but the fetch only happens once (well, twice locally because of strict mode but it would be once in prod):

export function BooklistContainer(): JSX.Element | null {
  const [term] = useTerm();
  const textbooks = useTextbooks(term);
  useEffect(() => {
    logEvent('textbooks_view', {term});
  }, [term]);

  return null;
}

I can also block the infinite fetch with a ref inside useTextbooks, but this still runs into infinite exceptions:

export const useTextbooks = (term: Term): TextbookResult => {
  const hasFetched = useRef(false);

  ...

  useEffect(() => {
    if (!hasFetched.current) {
      hasFetched.current = true;
      (async () => {
        ...
      })();
    }
  }, [term, termCourses]);

  return result;
};

Point being, it will take a bit of investigation but hopefully I was able to point you in a decent direction.

To Reproduce Steps to reproduce the behaviour/bug/issue:

1. Try to go to the textbook page
2. Do it with courses selected and without

Expected Behaviour Should work lol

Desktop/Mobile

• OS: MacOs Ventura
• Browser: Chrome
• Version: 113.0.5672.126

Additional Context I'm happy to look into this further later this week

[aomi]

@gawlster I think you're on the right track overall.

336 follows a PR that makes it so the terms are persisted. In the code snippets above there's some code that calls this which might be related. Perhaps removed the useTerm hook and hard coding it could uncover something.

[szeckirjr]

will close this ticket for now as PR #486 rolled back the useTerm changes and it now seems to be working

feel free to open it up again if not!