solidjs / solid-testing-library

Simple and complete Solid testing utilities that encourage good testing practices.
MIT License
201 stars 19 forks source link

location option ignored #65

Open tslocke opened 4 months ago

tslocke commented 4 months ago

This might be related to #60 - I'm also using vitest.

The following test fails:

import { render, screen} from '@solidjs/testing-library'
import { Route, Router } from "@solidjs/router"

test.only('render location', async () => {
  render(Component, {location: '/foo'})
  expect(await screen.findByText('foo')).toBeTruthy()
})

function Component() {
  return (
    <Router>
      <Route path='/' component={() => <p>root</p>}/>
      <Route path='/foo' component={() => <p>foo</p>}/>
    </Router>
  )
}

If I change the findByText to 'root' it passes. Am I using the API correctly?

atk commented 4 months ago

Actually, the router is already supplied by the test, so wrapping it in another router could break it.

tslocke commented 4 months ago

Does that mean I can't test my top-level <App/> component (which contains a router)?

Also this still fails for me - any advice?

test('render location', async () => {
  render(() => <Route path='/foo' component={() => <p>foo</p>}/>, {location: '/foo'})
  expect(await screen.findByText('foo')).toBeTruthy()
})

Thanks for your help

atk commented 4 months ago

That is strange, because this is very close to what we do in our tests: https://github.com/solidjs/solid-testing-library/blob/main/src/__tests__/routes.tsx#L18

tslocke commented 4 months ago

I've created a minimal repo with my test above and the test you linked. Both are failing for me

https://github.com/tslocke/solid-router-issue

atk commented 4 months ago

Thanks for the repro. I'll test it later.

tslocke commented 4 months ago

Any update on this? I'm unable to upgrade to the latest router because I can't get tests passing.

atk commented 4 months ago

Sorry, I updated our testing library and everything worked as expected. Maybe the references to older versions broke something, so I'll release the updated version right away.

atk commented 4 months ago

OK, I just published 0.8.9 and tested a bit more. Encapsulated routers do not yet work. I'm not sure if I will be able to support them, because we need an in-memory router for that and I don't know if I can inject it by default.

tslocke commented 4 months ago

Thanks very much for looking at this. I updated all the deps in that sample repo (and pushed) and the tests still fail for me. Is that expected? I'm not sure what you mean by "encapsulated" routers.

atk commented 4 months ago

Basically, testing your app.tsx. I still want to enable this, but the current approach does not work. Mocking the router is also not an option, because then I would either need to write my own mocking facilities or have to narrow down our use to one testing framework.

tslocke commented 4 months ago

OK. Not a problem for me at the moment. But right now I'm stuck on @solidjs/router 0.8 because the new router only supports things like useLocation when the component is inside a route, and as soon as I put things in a route the testing library doesn't render anything, e.g. this fails

test('render location', async () => {
  render(() => <Route path='/foo' component={() => <p>foo</p>}/>, {location: '/foo'})
  expect(await screen.findByText('foo')).toBeTruthy()
})

Repro: https://github.com/tslocke/solid-router-issue

atk commented 4 months ago

You can wrap useLocation in catchError in order to make the component work outside of the router. I'm sorry, but I cannot possibly support multiple versions of solid-router.

tslocke commented 4 months ago

Sorry for the confusion. I understand that's not supported. When I opened the issue I was trying to add a <Router> but you said that's not supported so I've switched to just a <Route> in my test, not a <Router>, and it's still not working. The repro I posted doesn't have a <Router> inside the test.

The docs say that is supported, e.g. (from https://github.com/solidjs/solid-testing-library):

it('uses params', async () => {
  const App = () => (
    <>
      <Route path="/ids/:id" component={() => <p>Id: {useParams()?.id}</p>} />
      <Route path="/" component={() => <p>Start</p>} />
    </>
  ); 
  const { findByText } = render(() => <App />, { location: "ids/1234" });
  expect(await findByText("Id: 1234")).not.toBeFalsy();
});
egtwp commented 4 months ago

Hello @atk , I'm looking into this because I'm facing the same problem!

I believe that the issue might be with the solidjs+vite+vitest template that you also suggest using in the install section of the README.

As soon as I try to simply copy your routes.tsx test file in a vanilla project created from that template (with just the router installed additionally), all the test that include the location prop of the render function fail. They print the following error on stdout:

Error attempting to initialize @solidjs/router:
"Unknown file extension ".jsx" for /workspaces/barebones-nodejs/node_modules/.pnpm/@solidjs+router@0.14.1_solid-js@1.8.11/node_modules/@solidjs/router/dist/index.jsx"

I encounter the same behavior in the repo linked to this issue, and I can also see the same error in issue #60 , which uses the same template.

Have you any idea about why vite integration could broke this? Can you suggest a different template/stack to use to get this to work?

Thank you in advance!

atk commented 4 months ago

This is a module loading issue (we had a lot of those already). Maybe try making the router an external import.

tslocke commented 3 months ago

Can you explain a bit more? I thought external imports were part of the build config and therefore would not affect testing?

atk commented 3 months ago

vitest internally uses vite to transform the files loaded. However, it has multiple mechanisms to load files - via node modules resolution or its internal resolver. Whenever file transformation fails in vitest, it's usually due to a mix-up between those modes (or in case of solid because it gets resolved twice over the different modes - solid may only be loaded once).

egtwp commented 3 months ago

@atk but the problem is only fired with solid-testing-library embedded router.

look at this:

import { createSignal, catchError } from "solid-js";
import { render, fireEvent } from "@solidjs/testing-library";
import { A, Route, Router, useParams } from "@solidjs/router";

describe("location option", () => {
  const Ids = () => (
    <>
      <p>Id: {useParams()?.id}</p>
      <p>
        <A href="/ids/9999" noScroll>
          navigate
        </A>
      </p>
    </>
  );
  const App = () => (
    <>
      <Route path="/ids/:id" component={Ids} />
      <Route path="/" component={() => <p>Start</p>} />
    </>
  );

  it("can render the main route with embedded router", async () => {
    const { findByText } = render(() => <App />, { location: "/" });
    expect(await findByText("Start")).not.toBeFalsy();
  });

  it("can render the main route with manually loaded router", async () => {
    const { findByText } = render(() => <Router><App /></Router>);
    expect(await findByText("Start")).not.toBeFalsy();
  });

});
stderr | src/routerino.test.tsx > location option > can render the main route with embedded router
Error attempting to initialize @solidjs/router:
"Unknown file extension ".jsx" for /workspaces/Saturday/node_modules/@solidjs/router/dist/index.jsx"

 ❯ src/routerino.test.tsx (2) 1051ms
   ❯ location option (2) 1050ms
     × can render the main route with embedded router 1024ms
     ✓ can render the main route with manually loaded router
egtwp commented 3 months ago

In my env adding this conf to vitest seems to resolve the issue without break any other tests

server: {
  deps: {
    inline: ["@solidjs/testing-library", "@solidjs/router"],
  }
}

how this sounds to you? I don’t know if could be anyway to not have to add this config manually, what do u think about?

let me know if I can help in any way.

tslocke commented 3 months ago

Did this get fixed? I've finally come back to upgrading the my router dep to 0.14 and I'm not having problems any more. It works even without the deps.inline config from @egtwp

atk commented 3 months ago

It cannot be fixed in testing-library, but I am preparing a fix in vite-plugin-solid.

tslocke commented 3 months ago

I'm no longer seeing that Unknown file extension ".jsx". No idea why. Can't complain : ) But I won't be surprised if it shows up again.

Maybe this issue should be closed, or perhaps renamed to track the issue you are working on in vite-plugin-solid? Up to you.

atk commented 3 months ago

I can influence the testing config from the vite-plugin-solid, but not from @solidjs/testing-library.