fix: Reset atoms when param is removed from URL when queryParams with param is specified

[AkifumiSato]

sumary

resolve: #1900
If queryParams and param are specified and there are no parameters in the URL, the update process may not be performed.
The reason is that null is assigned here, and the branch is not passed.

note

I was going to write a test, but the current test relies on RecoilSync_MockURLSerialization.js, so it is difficult to write a reproducible test for this bug.

[drarmstr]

@AkifumiSato - That mock URL serialization doesn't need to necessarily be used by all tests or can be extended. There is also a browser interface abstraction that may be useful for testing as well.

[AkifumiSato]

@drarmstr Yes, I tried to write a test without mock, but it did not work.
This is the diff when I tried it on another branch.
AkifumiSato@a58eb75#diff-74f521e2004fc6640581dbfffa78e3cf816c4b8b55c81ab8f58d21873af6981cR193

This is what I found when I tried this test case.

• pushState after rendering in a test case does not change the rendering(fail L.197).
• await flushPromisesAndTimers causes URL to revert back to before push (fail L.196).

For these reasons, I gave up implementing this tests.
Maybe I am not doing it right, but I could not figure out the cause.
Would you advise?

[facebook-github-bot]

@drarmstr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[drarmstr]

I think that's just because actions like history.pushState() don't trigger a popstate browser event. (See #1975). I've added the following updated test to the PR (No need for you to update the PR)

test('Remove parameter', async () => {
  const loc = {part: 'queryParams', param: 'param'};

  const atomA = atom({
    key: 'recoil-url-sync remove param',
    default: 'DEFAULT',
    effects: [syncEffect({itemKey: 'item', refine: string()})],
  });

  const container = renderElements(
    <TestURLSync location={loc}>
      <ReadsAtom atom={atomA} />
    </TestURLSync>,
  );
  expect(container.textContent).toBe('"DEFAULT"');

  history.replaceState(null, '', encodeURL([[loc, {item: 'SET'}]]));
  history.pushState(null, 'void');
  history.back();
  await flushPromisesAndTimers();
  expect(container.textContent).toBe('"SET"');

  // clear all query params from the URL to confirm it resets the atoms.
  history.replaceState(null, '', location.origin);
  history.pushState(null, 'void');
  history.back();
  await flushPromisesAndTimers();
  expect(container.textContent).toBe('"DEFAULT"');
});

[mondaychen]

This is strange. Why does this PR has no CI results?

[drarmstr]

This is strange. Why does this PR has no CI results?

Because it was an external submission which I then extended with tests and test fixes in www, but because it originated with the external PR I can't export those edits back out to the PR again to run the CI. Seems like a limitation in our syncing mechanism.