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

Client tests appear to be broken on new apps (beta 22) #4065

Closed
jhonnymichel opened this issue Jan 20, 2023 · 8 comments · Fixed by #4072
Closed

Client tests appear to be broken on new apps (beta 22) #4065

jhonnymichel opened this issue Jan 20, 2023 · 8 comments · Fixed by #4072
Labels
kind/bug Something isn't working status/done

Comments

@jhonnymichel
Copy link
Contributor

jhonnymichel commented Jan 20, 2023

What is the problem?

In beta 22, the vitest client tests are broken in a new app (blitz new).

The only frontend test that exists is initially skipped. If I remove the "skip" flag on test/index.test.tsx and run the test, it fail with this error:

Error: NextRouter was not mounted. https://nextjs.org/docs/messages/next-router-not-mounted
 ❯ Object.useRouter node_modules/src/client/router.ts:135:11
 ❯ BlitzProvider node_modules/@blitzjs/next/dist/chunks/index-browser.cjs:462:27
 ❯ renderWithHooks node_modules/react-dom/cjs/react-dom.development.js:16305:18
 ❯ mountIndeterminateComponent node_modules/react-dom/cjs/react-dom.development.js:20074:13
 ❯ beginWork node_modules/react-dom/cjs/react-dom.development.js:21587:16
 ❯ beginWork$1 node_modules/react-dom/cjs/react-dom.development.js:27426:14
 ❯ performUnitOfWork node_modules/react-dom/cjs/react-dom.development.js:26560:12
 ❯ workLoopSync node_modules/react-dom/cjs/react-dom.development.js:26466:5
 ❯ renderRootSync node_modules/react-dom/cjs/react-dom.development.js:26434:7
 ❯ recoverFromConcurrentError node_modules/react-dom/cjs/react-dom.development.js:25850:20

Looks like there's something wrong in BlitzProvider. The error happens inside of it, even when trying to render it inside <RouterContext>.

It's a clean install so I believe the scaffold for frontend tests could be broken.

is it possible that the test/utils files generated with blitz new is outdated? The file is different from this one I found in Blitz sourcecode: https://github.com/blitz-js/blitz/blob/main/integration-tests/utils/blitz-test-utils.tsx

Paste all your error logs here:

Error: NextRouter was not mounted. https://nextjs.org/docs/messages/next-router-not-mounted
 ❯ Object.useRouter node_modules/src/client/router.ts:135:11
 ❯ BlitzProvider node_modules/@blitzjs/next/dist/chunks/index-browser.cjs:462:27
 ❯ renderWithHooks node_modules/react-dom/cjs/react-dom.development.js:16305:18
 ❯ mountIndeterminateComponent node_modules/react-dom/cjs/react-dom.development.js:20074:13
 ❯ beginWork node_modules/react-dom/cjs/react-dom.development.js:21587:16
 ❯ beginWork$1 node_modules/react-dom/cjs/react-dom.development.js:27426:14
 ❯ performUnitOfWork node_modules/react-dom/cjs/react-dom.development.js:26560:12
 ❯ workLoopSync node_modules/react-dom/cjs/react-dom.development.js:26466:5
 ❯ renderRootSync node_modules/react-dom/cjs/react-dom.development.js:26434:7
 ❯ recoverFromConcurrentError node_modules/react-dom/cjs/react-dom.development.js:25850:20

What are detailed steps to reproduce this?

  1. Make sure you have the latest blitz beta installed globally
  2. Run blitz new my-app
  3. After the app is generated, cd to the project folder, install any packages needed to run the tests (I had to manually install vite after the app was generated)
  4. Run npm test or yarn test

Run blitz -v and paste the output here:

Blitz version: 2.0.0-beta.22 (global)
Blitz version: 2.0.0-beta.22 (local)
macOS Monterey | darwin-arm64 | Node: v16.14.0


 Package manager: npm

  System:
    OS: macOS 12.4
    CPU: (8) arm64 Apple M1
    Memory: 120.48 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.14.0/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
  npmPackages:
    @blitzjs/auth: 2.0.0-beta.22 => 2.0.0-beta.22
    @blitzjs/next: 2.0.0-beta.22 => 2.0.0-beta.22
    @blitzjs/rpc: 2.0.0-beta.22 => 2.0.0-beta.22
    @prisma/client: 4.6.0 => 4.6.0
    blitz: 2.0.0-beta.22 => 2.0.0-beta.22
    next: 13.1 => 13.1.3
    prisma: 4.6.0 => 4.6.0
    react: 18.2.0 => 18.2.0
    react-dom: 18.2.0 => 18.2.0
    typescript: ^4.8.4 => 4.9.4
@jhonnymichel jhonnymichel added kind/bug Something isn't working status/triage labels Jan 20, 2023
@jhonnymichel
Copy link
Contributor Author

https://github.com/blitz-js/blitz/blob/main/packages/blitz-next/src/index-browser.tsx#L83

Could this be the cause? useRouter is being called outside of the RouterContext.

@jhonnymichel
Copy link
Contributor Author

jhonnymichel commented Jan 27, 2023

export const mockRouter: NextRouter = {
  basePath: "",
  pathname: "/",
  route: "/",
  asPath: "/",
  params: {},
  query: {},
  isReady: true,
  isLocaleDomain: false,
  isPreview: false,
  push: vi.fn(),
  replace: vi.fn(),
  reload: vi.fn(),
  back: vi.fn(),
  prefetch: vi.fn(),
  beforePopState: vi.fn(),
  events: {
    on: vi.fn(),
    off: vi.fn(),
    emit: vi.fn(),
  },
  isFallback: false,
  forward: vi.fn(),
}

export const BlitzProvider = ({
  client = globalThis.queryClient,
  contextSharing = false,
  dehydratedState,
  hydrateOptions,
  children,
}: BlitzProviderProps) => {
  const router = mockRouter

  if (client) {
    return (
      <RouterContext.Provider value={router}>
        <QueryClientProvider
          client={client || globalThis.queryClient}
          contextSharing={contextSharing}
        >
          <Hydrate state={dehydratedState} options={hydrateOptions}>
            {children}
          </Hydrate>
        </QueryClientProvider>
      </RouterContext.Provider>
    )
  }

  return <RouterContext.Provider value={router}>{children}</RouterContext.Provider>
}

Mocking the router instead of using useRouter worked. I'm just not sure if that's the proper solution.

BTW, this mock object comes from Blitz itself. it's used in the toolkit tests: https://github.com/blitz-js/blitz/blob/main/apps/toolkit-app/test/utils.tsx#L77

Shouldn't the public BlitzProvider do the same?

@jhonnymichel
Copy link
Contributor Author

I think I found something else that is breaking tests right now:

https://github.dev/blitz-js/blitz/blob/main/packages/blitz-rpc/src/data-client/react-query-utils.ts#87

JEST_WORKER_ID will be undefined when using vitest. so this will return true and rendering pages with useMutation or useQuery will trigger this error thrown in validateQueryFn:

Either the file path to your resolver is incorrect (must be in a "queries" or "mutations" folder that isn't nested inside "pages" or "api") or you are trying to use Blitz's useQuery to fetch from third-party APIs (to do that, import useQuery directly from "@tanstack/react-query").`,

@flybayer
Copy link
Member

main/packages/blitz-next/src/index-browser.tsx#L83

Could this be the cause? useRouter is being called outside of the RouterContext.

yeah, this code is a bit screwy. We should not be rendering RouterContext in this code

@flybayer
Copy link
Member

I think I found something else that is breaking tests right now:

github.dev/blitz-js/blitz/blob/main/packages/blitz-rpc/src/data-client/react-query-utils.ts#87

JEST_WORKER_ID will be undefined when using vitest. so this will return true and rendering pages with useMutation or useQuery will trigger this error thrown in validateQueryFn:

Either the file path to your resolver is incorrect (must be in a "queries" or "mutations" folder that isn't nested inside "pages" or "api") or you are trying to use Blitz's useQuery to fetch from third-party APIs (to do that, import useQuery directly from "@tanstack/react-query").`,

the next line is checking for VITEST_WORKER_ID. Is that not being set?

@jhonnymichel
Copy link
Contributor Author

jhonnymichel commented Jan 27, 2023

I think I found something else that is breaking tests right now:
github.dev/blitz-js/blitz/blob/main/packages/blitz-rpc/src/data-client/react-query-utils.ts#87
JEST_WORKER_ID will be undefined when using vitest. so this will return true and rendering pages with useMutation or useQuery will trigger this error thrown in validateQueryFn:

Either the file path to your resolver is incorrect (must be in a "queries" or "mutations" folder that isn't nested inside "pages" or "api") or you are trying to use Blitz's useQuery to fetch from third-party APIs (to do that, import useQuery directly from "@tanstack/react-query").`,

the next line is checking for VITEST_WORKER_ID. Is that not being set?

The thing is, if JEST_WORKER_ID is not there, the function returns true already so it never checks for VITEST_WORKER_ID.

@jhonnymichel
Copy link
Contributor Author

So according from your first comment @flybayer and if I understood the second issue correctly, the fix here is:

  1. remove the rendering of RouterContext and the call to useRouter from BlitzProvider.
  2. update isNotInUserTestEnvironment to check if any of the 3 env variables are present instead of returning as soon as the JEST variable is not found.

I'd love to work on this after I have a confirmation that this is the intended solution

@jhonnymichel
Copy link
Contributor Author

^ ended up opening a PR, I believe the solution makes sense. let's see!

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

Successfully merging a pull request may close this issue.

3 participants