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

Relative URL doesn't work with appHandler: "Invalid URL" error #1000

Closed
rorysaur opened this issue Feb 23, 2024 · 4 comments
Closed

Relative URL doesn't work with appHandler: "Invalid URL" error #1000

rorysaur opened this issue Feb 23, 2024 · 4 comments
Labels
bug Something isn't working released

Comments

@rorysaur
Copy link

The problem

It seems this commit about normalization of URLs may have broken the ability to use a relative-style url param with the app handler because calling new URL(url) on a path like "/my-url" throws Invalid URL error (as one would expect it to, since new URL() doesn't accept relative URLs).

    TypeError: Invalid URL

      at normalizeUrlForceTrailingSlashIfPathnameEmpty (node_modules/.pnpm/next-test-api-route-handler@4.
0.3_next@14.1.0/node_modules/next-test-api-route-handler/dist/src/index.js:337:16)

I was able to get around this easily by providing a dummy beginning of the url as the tests do (e.g. "ntarh://api/books" instead of just "/api/books"), but it doesn't match what the docs say, so it required some digging to figure this out.

I noticed the tests include a case with url: '/my-url' and pagesHandler, but not a case for the same url with appHandler. I believe that's the case that's broken.

Reproduction steps
  1. Run a basic example like:
it("works", async () => {
  await testApiHandler({
    appHandler,
    url: "/my-url",
    async test({ fetch }) {
      const res = await fetch({
        method: "GET",
      })

      const json = await res.json()

      expect(json).toStrictEqual([])
    },
  })
})
@rorysaur rorysaur added the bug Something isn't working label Feb 23, 2024
@rorysaur
Copy link
Author

Wow, lucky #1000! 🌟

@Xunnamius
Copy link
Owner

Xunnamius commented Feb 23, 2024

Hmm. The Pages Router doesn't use the URL class and so accepts those relative style urls, but the App Router seems to exclusively use the URL class, which explains that discrepancy. As you said, the URL class requires an absolute url, which is why that ntarh:// version works (it could also be https://).

I'll go over the documentation to make sure it's clearer. I haven't used the App Router in production yet, but it doesn't seem like the App Router itself supports relative urls (i.e. all urls seem to be some instance of the URL class and/or NextUrl). If this assumption isn't accurate, I'll have to address that as well :)

@rorysaur
Copy link
Author

Ah! If it is unsupported, then a note in the docs to say something like, "to send query params, use literally any URL string with your query params at the end of it (ex: https://anything?foo=bar)" would have solved my use case. 👍🏻

Xunnamius added a commit that referenced this issue Mar 2, 2024
### [4.0.4](v4.0.3...v4.0.4) (2024-03-02)

#### 🪄 Fixes

* **src:** allow relative url strings passed via url shorthand for App Router ([01b86b6](01b86b6)) <sup>closes [#1000](https://github.com/Xunnamius/next-test-api-route-handler/issues/1000)</sup>
* **src:** prevent recursive redirection with undici/whatwg fetch ([22bb716](22bb716)) <sup>closes [#993](https://github.com/Xunnamius/next-test-api-route-handler/issues/993)</sup>
* **src:** replace `AppRouteUserlandModule` with looser type ([502e666](502e666)) <sup>closes [#1006](#1006), [#1005](https://github.com/Xunnamius/next-test-api-route-handler/issues/1005)</sup>
@Xunnamius
Copy link
Owner

🎉 This issue has been resolved in version 4.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

2 participants