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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[馃悶] There are some tests is weird #6124

Open
JerryWu1234 opened this issue Apr 16, 2024 · 2 comments
Open

[馃悶] There are some tests is weird #6124

JerryWu1234 opened this issue Apr 16, 2024 · 2 comments
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working

Comments

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented Apr 16, 2024

Which component is affected?

Qwik Playground

Describe the bug

https://github.com/JerryWu1234/qwik/blob/16ac4a5c3d232a29d2d35653ffd70b6a0836aca9/starters/apps/qwikcity-test/src/routes/issue2890/a/index.tsx#L15

<Link
          id="issue2890-link-1"
          href="/qwikcity-test/issue2890/b/?query=123"
        >
          /b/?query=123
</Link>

entry.ssr.tsx

import {
  renderToStream,
  type RenderToStreamOptions,
} from "@builder.io/qwik/server";
import { manifest } from "@qwik-client-manifest";
import Root from "./root";

export default function (opts: RenderToStreamOptions) {
  return renderToStream(<Root />, {
    manifest,
    base: "/qwikcity-test/build/",
    ...opts,
  });
}

Every test has the base URL added manually in the QwikCity test, but in the real scenario, the user's access URL could automatically add the base when the user sets it

Reproduction

https://github.com/QwikDev/qwik/tree/main/starters/e2e

Steps to reproduce

pnpm run dev

System Info

System:
    OS: macOS 14.2.1
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
    Memory: 102.11 MB / 8.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 10.2.4 - ~/.nvm/versions/node/v20.11.1/bin/npm
    pnpm: 8.15.4 - ~/.nvm/versions/node/v20.11.1/bin/pnpm
  Browsers:
    Chrome: 123.0.6312.124
    Edge: 123.0.2420.97
    Safari: 17.2.1
  npmPackages:
    undici: ^5.12.0 => 5.12.0 
    vite: ^2.9.5 => 2.9.15

Additional Information

No response

@JerryWu1234 JerryWu1234 added STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working labels Apr 16, 2024
@thejackshelton
Copy link
Member

thejackshelton commented Apr 17, 2024

So you're saying the <Link /> component should automatically handle the inclusion of the base URL in the href? Or the fact that the tests include it in the link href should be removed?

On a new Qwik City project, adding the following seemed to work fine automatically:

  return renderToStream(<Root />, {
    manifest,
    ...opts,
    // Use container attributes to set attributes on the html tag.
    containerAttributes: {
      lang: "en-us",
      base: "/test-base/",
      ...opts.containerAttributes,
    },
  });
      <Link href="/about">TO ABOUT</Link>

would still take me to the about page.

image

@JerryWu1234
Copy link
Contributor Author

tests include it in the link href should be removed?

I think it should be removed. What do you think?

On the other hand.
If the user sets a base in vite.config.js. Perhaps there are some conflicts in both properties.

@PatrickJS PatrickJS changed the title There are some tests is weird [馃悶] [馃悶] There are some tests is weird May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants