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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested SSRProviders don't guarantee unique IDs #2277

Closed
jfuchs opened this issue Sep 2, 2021 · 1 comment 路 Fixed by #2278
Closed

Nested SSRProviders don't guarantee unique IDs #2277

jfuchs opened this issue Sep 2, 2021 · 1 comment 路 Fixed by #2278
Labels
bug Something isn't working

Comments

@jfuchs
Copy link
Contributor

jfuchs commented Sep 2, 2021

馃悰 Bug Report

Given this tree of SSR providers (cribbing from SSRProvider.test.js):

<SSRProvider>
  <SSRProvider>
    <Test />
    <SSRProvider>
      <Test />
    </SSRProvider>
  </SSRProvider>
  <SSRProvider>
    <Test />
  </SSRProvider>
</SSRProvider>

Each <Test>, when calling useId, should get a unique ID, but two of those tests do not.

馃 Expected Behavior

I don't have a preference for what the IDs are so long as they are unique and namespaced and deterministic.

馃槸 Current Behavior

Here's a snapshot test showing the current behavior:

it("it should generate consistent unique ids with nested SSR providers", function () {
  let tree = render(
    <SSRProvider>
      <SSRProvider>
        <Test />
        <SSRProvider>
          <Test />
        </SSRProvider>
      </SSRProvider>
      <SSRProvider>
        <Test />
      </SSRProvider>
    </SSRProvider>
  );

  let divs = tree.getAllByTestId("test");
  expect(divs).toMatchInlineSnapshot(`
    Array [
      <div
        data-testid="test"
        id="react-aria-1-1"
      />,
      <div
        data-testid="test"
        id="react-aria-2-1"
      />,
      <div
        data-testid="test"
        id="react-aria-2-1"
      />,
    ]
  `);
});

馃拋 Possible Solution

I believe a possible fix would be making each ID essentially a path through the tree of SSRProviders, so that for the above example, the IDs would be:

react-aria-1-1
react-aria-1-1-1
react-aria-1-2

馃敠 Context

I stumbled into this looking at different implementations for SSR-stable unique ID generation in react.

馃捇 Code Sample

See the current behavior above

馃實 Your Environment

I was running tests on main as of 2021 Aug 12 (v3.0.3 of @react-aria/ssr)

@ansavchenco
Copy link

ansavchenco commented Apr 9, 2022

I ran the forementioned test on the latests main and it failed.

// Provided tree
<SSRProvider>
  <Test />
  <SSRProvider>
    <Test />
    <SSRProvider>
      <Test />
    </SSRProvider>
  </SSRProvider>
  <Test />
  <SSRProvider>
    <Test />
  </SSRProvider>
</SSRProvider>
// Expected
[
  "react-aria-1",
  "react-aria-2-1",
  "react-aria-2-2-1",
  "react-aria-3",
  "react-aria-4-1",
]
// Actual
[
  'react-aria-1',
  'react-aria-1-1',
  'react-aria-1-1-1',
  'react-aria-2',
  'react-aria-2-1'
]

Can you please tell what behaviour is expected so that I can come up with a PR that fixes either the test or the behaviour? To me it seems that the actual behaviour is alright since all ids are unique. Also the logic behind generation in the actual result can be inferred just by looking and the code sample. In the expected result it's not clear why the first prefix is incremented in the second id but not incremented in the third id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants