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

location option ignored #65

Open
tslocke opened this issue Jul 17, 2024 · 23 comments
Open

location option ignored #65

tslocke opened this issue Jul 17, 2024 · 23 comments

Comments

@tslocke
Copy link

tslocke commented Jul 17, 2024

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
Copy link
Collaborator

atk commented Jul 17, 2024

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

@tslocke
Copy link
Author

tslocke commented Jul 17, 2024

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
Copy link
Collaborator

atk commented Jul 18, 2024

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
Copy link
Author

tslocke commented Jul 18, 2024

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
Copy link
Collaborator

atk commented Jul 18, 2024

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

@tslocke
Copy link
Author

tslocke commented Jul 22, 2024

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

@atk
Copy link
Collaborator

atk commented Jul 22, 2024

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
Copy link
Collaborator

atk commented Jul 22, 2024

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
Copy link
Author

tslocke commented Jul 22, 2024

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
Copy link
Collaborator

atk commented Jul 23, 2024

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
Copy link
Author

tslocke commented Jul 23, 2024

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
Copy link
Collaborator

atk commented Jul 23, 2024

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
Copy link
Author

tslocke commented Jul 23, 2024

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
Copy link

egtwp commented Jul 29, 2024

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/@[email protected][email protected]/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
Copy link
Collaborator

atk commented Jul 29, 2024

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

@tslocke
Copy link
Author

tslocke commented Jul 30, 2024

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

@atk
Copy link
Collaborator

atk commented Jul 30, 2024

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
Copy link

egtwp commented Jul 30, 2024

@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
Copy link

egtwp commented Jul 30, 2024

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
Copy link
Author

tslocke commented Aug 26, 2024

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
Copy link
Collaborator

atk commented Aug 26, 2024

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

@tslocke
Copy link
Author

tslocke commented Aug 26, 2024

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
Copy link
Collaborator

atk commented Aug 28, 2024

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

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

No branches or pull requests

3 participants