[🐞] Duplicate implementations of JSXNode Errors in RC, v0.103

[n8sabes]

Which component is affected?

Qwik Runtime

Describe the bug

I'm getting QWIK WARN Duplicate implementations of "JSXNode" found warnings when using Qwik Styled Vanilla Extract.

This happens with RC1 Starter, following Integration Docs. Please see Loom Example, if needed.

Also, with v1.0 around the corner, there are open PRs and Issues with styled-vanilla-extract that should get a little love since it's listed in the official documentation integrations section.

Cc: @manucorporat, @wmertens

Reproduction

https://www.loom.com/share/9eab1daa92f0431d848f35ea67899258

Steps to reproduce

pnpm create qwik@latest
pnpm qwik add styled-vanilla-extract
pnpm dev

System Info

System:
    OS: macOS 13.3.1
    CPU: (8) arm64 Apple M1
    Memory: 65.23 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.0.0 - ~/.nvm/versions/node/v20.0.0/bin/node
    npm: 8.19.2 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 112.0.5615.137
    Firefox Developer Edition: 111.0
    Safari: 16.4
  npmPackages:
    @builder.io/qwik: 0.103.0 => 0.103.0
    @builder.io/qwik-city: ~0.103.0 => 0.103.0
    undici: 5.21.2 => 5.21.2
    vite: 4.2.2 => 4.2.2

Additional Information

No response

[brandiqa]

I've come across this issue in v1.1.1 and v1.0.0. when using the library @qwikest/icons

[NiklasPor]

FYI @n8sabes , I've managed to "fix" this by merging all my original entry points into a single one. But after this the qwik compiler is too heavy for my machine and the build fails because it runs out of heap (~about 24k qwik components)

[wmertens]

@NiklasPor do you really need component$s? Or could lite components suffice?

Then maybe you can skip the build step and just export functions that call jsx?

[NiklasPor]

Hm, about one third of the components use a context hook and if I remember correctly they don't work in lite / inline components. I'll consider throwing away the feature which uses the context, so that I atleast can publish a stable version 👍

Still I think it would be great if we could maybe reach a point in the future, where it's possible to add multiple entrypoints, just like with a regular vite config / js package. A lot of large packages in the JS ecosystem use the functionality. Probably a possibility for later point on some roadmap 👀

[wmertens]

You could also only build one lite and one regular component and use those as templates for the rest

[NiklasPor]

Lite components seem to work fine. Just in dev mode (vite --mode ssr) no tree-shaking is happening, so on the first user interaction with one of the lite components all of the JS loaded. Preview mode tree-shaking works and performance is superb as always 😁

[NiklasPor]

Alright, so the icon package now works with multiple entrypoints + only lite components – but the warning is now back that I'm using multiple entrypoints. Thanks for the suggestions @wmertens

[hbendev]

The warning is still there when using @qwikest/icons 0.0.8 together with qwik 1.1.0 or qwik 1.1.4 and using import { HiXY } from "@qwikest/icons/heroicons".

[chempogonzalez]

Hi! I have seen the workaround commited from @kisaragi-hiu referencing this issue but it removes completely all errors thrown to the shell.

As a temporal workaround to avoid the noise caused using @qwikest/icons with all these warnings in console, I'm using the following script on top of the entry.ssr.tsx. By this way we don't loose other console outputs. I hope it helps you @hbendev :)

// entry.ssr.tsx
import { isDev } from '@builder.io/qwik/build'

if (isDev) {
  const consoleWarn = console.warn
  const SUPPRESSED_WARNINGS = ['Duplicate implementations of "JSXNode" found']

  console.warn = function filterWarnings (msg, ...args) {
    if (!SUPPRESSED_WARNINGS.some(entry => msg.includes(entry) || args.some(arg => arg.includes(entry))))
      consoleWarn(msg, ...args)
  }
}

[noc2spam]

Hi! I have seen the workaround commited from @kisaragi-hiu referencing this issue but it removes completely all errors thrown to the shell.

As a temporal workaround to avoid the noise caused using @qwikest/icons with all these warnings in console, I'm using the following script on top of the entry.ssr.tsx. By this way we don't loose other console outputs. I hope it helps you @hbendev :)

// entry.ssr.tsx
import { isDev } from '@builder.io/qwik/build'

if (isDev) {
  const consoleWarn = console.warn
  const SUPPRESSED_WARNINGS = ['Duplicate implementations of "JSXNode" found']

  console.warn = function filterWarnings (msg, ...args) {
    if (!SUPPRESSED_WARNINGS.some(entry => msg.includes(entry) || args.some(arg => arg.includes(entry))))
      consoleWarn(msg, ...args)
  }
}

Works fine, thank you. :)

However, I guess solving the problem actually would be a better solution at the end of the day. :)

[wmertens]

I'd like to add that it seems like the error is harmless in the case of the icons package, but it can lead to actual things not working, see #4436

[NiklasPor]

I'd like to add that it seems like the error is harmless in the case of the icons package, but it can lead to actual things not working, see #4436

I very strongly second this, the only reasons the icons package is working, is that I'm not using any component$ inside there, but instead use regular function components / lite components. Otherwise the package would break during server side rendering afaik.

If I'd need to guess, the reason seems to be connected to the multiple entrypoints. Probably qwik is only perprocessing specfic entrypoints?

[Mo0nbase]

Has there been any progress on this issue? I currently cannot use the icons package because It is breaking parts of my app.

[mhevery]

So this error means that Vite has somehow packaged up the JSX code twice! And this is an early error so that you don't go nuts later with strange behavior.... So if you see this error, the question you should be asking is why are there two copies of JSX implementation in the system. This is not an issue with Qwik, but rather how you have the vite configured....

If someone can give me a simple reproduction that you think should be fixed, I will look into it.

[wmertens]

@mhevery I posted a repro in #4436, https://stackblitz.com/edit/qwik-starter-wadzsa?file=src/routes/index.tsx

it happens in dev mode too, and it breaks in production. Qwik is definitely packaged only once. The error indicates that the JSXNodeImpl class is instantiated twice, but I can't figure out how.

[NiklasPor]

@mhevery I posted a repro in #4436, https://stackblitz.com/edit/qwik-starter-wadzsa?file=src/routes/index.tsx

it happens in dev mode too, and it breaks in production. Qwik is definitely packaged only once. The error indicates that the JSXNodeImpl class is instantiated twice, but I can't figure out how.

I'm pretty sure this happens, because you also use multiple named exports in the package.json. With @qwikest/icons I did not get any issues when I merged all code behind a single default export (sadly this wasn't an option for the package).

Maybe there's something hardcoded in Qwik which looks exactly for an entrypoint . like mentioned in the qwik lib docs?

    ".": {
      "import": "./lib/index.qwik.mjs",
      "require": "./lib/index.qwik.cjs",
      "types": "./lib-types/index.d.ts"
    }

The docs also impose some restrictions on how the file must be named:

The file must be called with the .qwik.mjs extension, otherwise the Qwik Optimizer will not recognize it.

In styled-vanilla-extract it looks like this:

    "./qwik": {
      "import": "./lib/index.js",
      "require": "./lib/index.cjs",
      "types": "./lib-types/index.d.ts"
    },
    "./vite": {
      "import": "./lib/vite.js",
      "require": "./lib/vite.cjs",
      "types": "./lib-types/vite.d.ts"
    },
    "./qwik-styled": {
      "import": "./lib/qwik-styled.js",
      "require": "./lib/qwik-styled.cjs"
    },
    "./package.json": "./package.json"

---

I would love it if we get to support multi-entrypoint packages.

[wmertens]

@NiklasPor well you just solved why qwik-ui doesn't work :sweat_smile: cc @shairez

I'll see if I can fix styled-vanilla-extract.

I'm assuming the duplicated JSXNodeImpl is because of the optimizer bundling it directly and then vite bundling it separately where it wasn't done by the optimizer.

If so, the optimizer could instead look at the imports and see if qwik is imported instead of only relying on the filename

[NiklasPor]

Always happy to help, I'd love it when we get the packages to work without problems – kinda important for the ecosystem 😁

[wmertens]

I upgraded the repro to latest everything and built styled-vanilla-extract with .qwik.mjs extensions, but it didn't help.

https://stackblitz.com/edit/qwik-starter-8kdqva?file=package-lock.json,package.json

[wmertens]

I figured it out. In the repro above I added console.log(new Error('loading qwik [prod] [mjs|cjs]')) in the node_modules qwik core files, and indeed qwik gets loaded multiple times.

What happens is: Vanilla-extract runs the css.ts files at build time and generates a file that imports styled-vanilla-extract/qwik-styled which in turn imports qwik. https://github.com/wmertens/styled-vanilla-extract/blob/4f03da0fc8d53d2f6bd19a52e523c4c7f30571ee/src/ve-style.ts#L44

This generated import doesn't get bundled by vite. You can see it at the top of server/entry.preview.mjs:

import { styled } from "styled-vanilla-extract/qwik-styled";

I added the vite inspect plugin to the repro so you can see all the transforms. Everything seems correct, but the dev build does import core.prod.mjs even though I can't find what imports it.

For dev mode, I assume the same thing is happening.

---

So the problem is either:

• vite is bundling qwik into the server entry, or
• vite is not bundling styled-vanilla-extract as well

I think it would be safer if the server build did not bundle qwik

[wmertens]

BTW @NiklasPor not that it seems to make a difference, but your package.json does not include the "qwik" exports which the docs say are required. (your file naming is ok)

[wmertens]

Ok I solved it.

This is what happens:

• the qwikVite plugin forces bundling of Qwik
• vanilla-extract generates an import from an external which doesn't get bundled
• but it contains an import of qwik
• so then it will have a duplicate entry of qwik
• duplicate spidermen pointing.jpeg
• ...and this is already thought of! In that same line, it includes vendorIds
• ...they are found by checking all the dependencies for package.json files that have qwik as a key!

So by having a key "qwik" at the root of your package.json, it will be bundled into the ssr build, which fixes the duplicate jsxnode.

This is somewhat documented, so it's a matter of making it more clear.

Ideally, the node_modules qwik is somehow poisoned with an error so that it can be caught and corrected easily.

I do think it's a bit confusing to have the "qwik" property be required and it not seemingly doing anything. It's also not documented what it needs to point to - what if you have multiple entry points for qwik code?

So @NiklasPor add the "qwik" attribute to your package.

@mhevery @manucorporat what should the "qwik" attribute point at, exactly? And how to make it easier to discover that ".qwik.js" wasn't generated or that the "qwik" attribute is missing?

[NiklasPor]

Awesome @wmertens, I'll check If I can finally fix the package with this! 😅

[NiklasPor]

@wmertens Seems like my local testing everything went fine with this, can't believe that this small thing made the change 🤯 I'll check if it works with the community, thanks a lot ❤️ 0.0.9 of @qwikest/icons includes the change, I'm just pointing to an empty entrypoint 😆

[mhevery]

@mhevery @manucorporat what should the "qwik" attribute point at, exactly? And how to make it easier to discover that ".qwik.js" wasn't generated or that the "qwik" attribute is missing?

Maybe I am misunderstanding the question, but I think the package.json should just be

{
  "qwik": true
}

[mhevery]

+1 to better document this. Also maybe add a section to the documentation explaining a common failure mode and then update the error with a pointer to the documentation saying maybe this is what is going on? Any chance @wmertens or someone could create a PR for that? Please 🍒 on top?

[wmertens]

Maybe I am misunderstanding the question, but I think the package.json should just be

{
  "qwik": true
}

I did try that but it complains somewhere. It expects a string. Presumably it's used elsewhere, the vendorIds just use it as a truthy value.

[mhevery]

What is the error?

[mhevery]

OK,

I think the code is here: https://github.com/BuilderIO/qwik/blob/main/packages/qwik/src/optimizer/src/plugins/vite.ts#L704 And the correct value should be a pointer to the MJS file: "qwik": "./lib/index.qwik.mjs",

[wmertens]

What is the error?

TypeError [ERR_INVALID_ARG_TYPE]: The "paths[1]" argument must be of type string. Received type boolean (true)
    at __node_internal_ (https://qwikstarter8kdqva-clg3.w-credentialless.staticblitz.com/blitz.5e765aa0.js:36:5410)
    at new <anonymous> (https://qwikstarter8kdqva-clg3.w-credentialless.staticblitz.com/blitz.5e765aa0.js:36:4172)
    at validateString (https://qwikstarter8kdqva-clg3.w-credentialless.staticblitz.com/blitz.5e765aa0.js:38:1232)
    at Object.resolve (https://qwikstarter8kdqva-clg3.w-credentialless.staticblitz.com/blitz.5e765aa0.js:44:7412)
    at eval (file:///home/projects/qwik-starter-8kdqva/node_modules/@builder.io/qwik/optimizer.cjs:3445:34)
    at Array.map (<anonymous>)
    at findQwikRoots (file:///home/projects/qwik-starter-8kdqva/node_modules/@builder.io/qwik/optimizer.cjs:3438:37)
    at async config (file:///home/projects/qwik-starter-8kdqva/node_modules/@builder.io/qwik/optimizer.cjs:3073:49)
    at async runConfigHook (file:///home/projects/qwik-starter-8kdqva/node_modules/vite/dist/node/chunks/dep-df561101.js:66324:25)
    at async resolveConfig (file:///home/projects/qwik-starter-8kdqva/node_modules/vite/dist/node/chunks/dep-df561101.js:65767:14)
    at async _createServer (file:///home/projects/qwik-starter-8kdqva/node_modules/vite/dist/node/chunks/dep-df561101.js:65011:20)
    at async CAC.eval (file:///home/projects/qwik-starter-8kdqva/node_modules/vite/dist/node/cli.js:758:24) {
  code: 'ERR_INVALID_ARG_TYPE'
}

...which is indeed inside the code you point at.

And the vendorRoots are used by the optimizer as input file: https://github.com/BuilderIO/qwik/blob/92b77bbe4dbbe623f26dcaf3165e3e1940b6b4af/packages/qwik/src/optimizer/src/optimizer.ts#L68

So it should be a single root file that encompasses all qwik code. Would be nicer as an array.

[mhevery]

Would be nicer as an array.

😁 PR ?