Skip to content

useParams() shows different behaviour between Routes and RouteProvider when using react-router 7.1.5 in library mode #12939

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

Open
almilo opened this issue Feb 3, 2025 · 3 comments

Comments

@almilo
Copy link

almilo commented Feb 3, 2025

I'm using React Router as a...

library

Reproduction

Based on this useParams() test:

describe("when the path has some params", () => {
I created the following tests to show the different behaviour:

import '@testing-library/jest-dom/vitest';
import { cleanup, render, screen } from '@testing-library/react';
import { afterEach, describe, expect, it } from 'vitest';
import { createMemoryRouter, MemoryRouter, Route, Routes, useParams } from 'react-router';
import { RouterProvider } from 'react-router/dom';

describe('useParams', () => {
  afterEach(cleanup);

  function ShowParams() {
    return <pre data-test="params">{JSON.stringify(useParams())}</pre>;
  }

  it('with routes shows the url params', async () => {
    render(
      <MemoryRouter initialEntries={['/blog/react-router']}>
        <Routes>
          <Route path="/blog/:slug" element={<ShowParams />} />
        </Routes>
      </MemoryRouter>,
    );

    expect(await screen.findByTestId('params')).toMatchInlineSnapshot(`
        <pre
          data-test="params"
        >
          {"slug":"react-router"}
        </pre>
      `);
  });

  it('with route provider shows the url params', async () => {
    const router = createMemoryRouter(
      [
        {
          path: '/blog/:slug',
          element: <ShowParams />,
        },
      ],
      {
        initialEntries: ['/blog/react-router'],
      },
    );

    render(<RouterProvider router={router} />);

    expect(await screen.findByTestId('params')).toMatchInlineSnapshot(`
      <pre
        data-test="params"
      >
        {"slug":"react-router"}
      </pre>
    `);
  });
});

System Info

Node version: v20.14.0

Package versions used:
    "react": "^18.3.1",
    "react-dom": "^18.3.1",
    "react-router": "^7.1.5",

    "@testing-library/jest-dom": "^6.5.0",
    "@testing-library/react": "^14.3.1",
    "@testing-library/user-event": "^14.5.2",
    "vitest": "^1.6.0"

Used Package Manager

npm

Expected Behavior

Hi React Router Team,

While following the migration guide in https://reactrouter.com/upgrading/v6 and after switching from v6.x to v7.x a non-documented breaking change in useParams() seems to be reproducible in unit tests. The current react-router useParams() unit tests do not cover the usage of RouteProvider explicitly and also use the deprecated "react-test-renderer" package. I would expect to have this behaviour mentioned in the migration guide (if not a bug) and some up-to-date tests to show the required changes between RouteProvider and createBrowserRouter().

Thanks a lot in advance!

Actual Behavior

When executing the test above with "[email protected]", the test using the route provider fails as useParams() does not return any parameters:
Image
Image

When executing the test above with "[email protected]" and "[email protected]", both tests pass:
Image

Using all future flags does not fix the failing test either:

    const router = createMemoryRouter(
      [
        {
          path: '/blog/:slug',
          element: <ShowParams />,
        },
      ],
      {
        initialEntries: ['/blog/react-router'],
        future: {
          v7_relativeSplatPath: true,
          v7_startTransition: true,
          v7_fetcherPersist: true,
          v7_normalizeFormMethod: true,
          v7_partialHydration: true,
          v7_skipActionErrorRevalidation: true,
        },
      },
    );
@almilo almilo added the bug label Feb 3, 2025
@almilo
Copy link
Author

almilo commented Feb 13, 2025

After further investigation, it seems that this little change:
Image
made the test pass:
Image
The different behaviour between the two imports is completely unexpected and error-prone.

@niklas-lumio-aiven
Copy link

@almilo This is documented in https://reactrouter.com/upgrading/v6 as the last step. Though very error prone indeed and ideally should be changed if possible to avoid confusion on the matter.

@almilo
Copy link
Author

almilo commented Feb 17, 2025

@niklas-lumio-aiven thanks a lot for the clarification. I agree that this is documented and I failed to follow / understand that last step properly. Given the thousands of developers who will be following this guide in the near future, I would suggest to:

  • detect if possible in the code when the import is being used in a non-DOM environment and show a console error / warning to raise the awareness (similar to the future flags).
  • if not possible, raise the relevance of this point with a warning in the documentation to make clear that using the wrong import in non-DOM contexts will / could fail (documentation equivalent to the previous execution warning).

Explaining why the behaviour is different and what consequences could have is always helpful for self-learning. The current message is at the moment not relevant enough IMO:
Image
It fails to convey that this is not a convenience change (I wrongly thought so) but a behavioural difference.

Thanks a lot for the feedback.

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

No branches or pull requests

3 participants