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

[🐞] Support Typescript "moduleResolution": "node16" and "bundler" #4689

Open
ziimakc opened this issue Jul 2, 2023 · 21 comments
Open

[🐞] Support Typescript "moduleResolution": "node16" and "bundler" #4689

ziimakc opened this issue Jul 2, 2023 · 21 comments
Labels
COMP: optimizer TYPE: bug Something isn't working

Comments

@ziimakc
Copy link

ziimakc commented Jul 2, 2023

Which component is affected?

Qwik Rollup / Vite plugin

Describe the bug

When using esm import .js or .jsx extensions is required. Trying to follow this requirement qwik fails to build with the following error:

Could not resolve "./components/router-head/router-head.jsx" from "src/s_qn6vsz8sp28.js"

error during build:
RollupError: Could not resolve "./components/router-head/router-head.jsx" from "src/s_qn6vsz8sp28.js"
    at error (file:///home/app/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/node-entry.js:2213:30)
    at ModuleLoader.handleInvalidResolvedId (file:///home/app/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/node-entry.js:24219:24)
    at file:///home/app/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/es/shared/node-entry.js:24181:26
// root.tsx
import { RouterHead } from "./components/router-head/router-head.jsx";

Reproduction

  • git clone https://github.com/ZiiMakc/temp
  • npm install
  • npm run build

Steps to reproduce

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.1 LTS 22.04.1 LTS (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Core(TM) i5-9300H CPU @ 2.40GHz
    Memory: 5.29 GB / 7.68 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.16.1 - /usr/bin/node
    npm: 9.5.1 - /usr/bin/npm

Additional Information

No response

@ziimakc ziimakc added TYPE: bug Something isn't working STATUS-1: needs triage New issue which needs to be triaged labels Jul 2, 2023
@manucorporat
Copy link
Contributor

When using esm import .js or .jsx extensions is required

Not sure i understand

@manucorporat
Copy link
Contributor

Tried and it works, can you provide a full repo that reprodices the issue?

@manucorporat manucorporat added the STATUS-2: needs reproduction Issue lacks reproduction label Jul 2, 2023
@ziimakc
Copy link
Author

ziimakc commented Jul 2, 2023

@manucorporat

Here is example:

  • git clone https://github.com/ZiiMakc/temp
  • npm install
  • npm run build

Problem is because of .jsx suffix import { RouterHead } from "./components/router-head/router-head.jsx";

@ziimakc
Copy link
Author

ziimakc commented Jul 2, 2023

Probably related: rollup/rollup#4917

@ziimakc
Copy link
Author

ziimakc commented Jul 5, 2023

See also vitejs/vite#8993

@ziimakc ziimakc changed the title [🐞] Esm extensions not working [🐞] Support Typescript 4.7 "Node16" extension requirements Jul 5, 2023
@ziimakc
Copy link
Author

ziimakc commented Jul 5, 2023

Temporary solution:

import { defineConfig } from "vite";
import { qwikVite } from "@builder.io/qwik/optimizer";
import { qwikCity } from "@builder.io/qwik-city/vite";
import pluginTypescript from "@rollup/plugin-typescript";

export default defineConfig(() => {
  return {
    plugins: [qwikCity(), qwikVite()],
    build: {
      rollupOptions: {
        plugins: [pluginTypescript()],
      },
    },
  };
});

Also remove outDir and declaration from tsconfig

@octet-stream
Copy link
Contributor

I have the same issue. Here's minimal repro on StackBlitz: https://stackblitz.com/edit/qwik-starter-rfjklh?file=src%2Froot.tsx

Just run npm run build in the terminal and you'll get the error.

@octet-stream
Copy link
Contributor

octet-stream commented Jan 24, 2024

From what I can tell, it is possible to use .js extensions when importing modules within .ts files. At least this is possible for server-side code. You can see that in src/routes/plugin.ts file: if you remove .js from src/root.tsx, but keep one in src/routes/plugin.tsx then npm run build will build the project successfully. I have tried to import the same file (src/lib/add) in home page module. It shows the error even with npm run dev if I use add function imported from this module in useTask$. On the server this code will be imported and executed without any problem, but then on the client it doesn't work.

@PatrickJS
Copy link
Member

PatrickJS commented May 6, 2024

̶t̶h̶i̶s̶ ̶s̶e̶e̶m̶s̶ ̶t̶o̶ ̶w̶o̶r̶k̶ ̶n̶o̶w̶ ̶a̶s̶ ̶w̶e̶ ̶a̶r̶e̶ ̶o̶n̶ ̶d̶i̶f̶f̶e̶r̶e̶n̶t̶ ̶T̶y̶p̶e̶S̶c̶r̶i̶p̶t̶ ̶v̶e̶r̶s̶i̶o̶n̶ ̶a̶n̶d̶ ̶n̶o̶d̶e̶ ̶r̶e̶q̶u̶i̶r̶e̶m̶e̶n̶t̶ ̶n̶o̶w̶.̶ ̶p̶l̶e̶a̶s̶e̶ ̶m̶a̶k̶e̶ ̶a̶ ̶n̶e̶w̶ ̶i̶s̶s̶u̶e̶ ̶i̶f̶ ̶t̶h̶e̶r̶e̶ ̶i̶s̶ ̶s̶t̶i̶l̶l̶ ̶a̶n̶ ̶i̶s̶s̶u̶e̶

@PatrickJS PatrickJS closed this as not planned Won't fix, can't repro, duplicate, stale May 6, 2024
@octet-stream
Copy link
Contributor

@PatrickJS is there any estimate date for this fix to be released? Because I still get the same error in 1.5.3, so it's not fixed yet.

@PatrickJS PatrickJS reopened this May 7, 2024
@PatrickJS
Copy link
Member

PatrickJS commented May 7, 2024

I need a repro repo the one provided is not public and is it only this version of TypeScript or can we use latest version?

@octet-stream
Copy link
Contributor

@PatrickJS
Copy link
Member

well it's .tsx and you need allowImportingTsExtensions: true in tsconfig

@octet-stream
Copy link
Contributor

octet-stream commented May 7, 2024

This doesn't solve the issue, I just get different error if I replace .js with .ts/.tsx:

vite v5.2.11 building for production...
✓ 46 modules transformed.
/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:10103
    throw new Error("start < 0");
    ^

Error: start < 0
    at createTextSpan (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:10103:11)
    at createTextSpanFromBounds (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:10111:10)
    at getErrorSpanForNode (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:12660:10)
    at createDiagnosticForNodeFromMessageChain (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:12497:16)
    at resolveExternalModule (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:47034:31)
    at resolveExternalModuleNameWorker (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:46928:61)
    at resolveExternalModuleName (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:46925:12)
    at getSymbolAtLocation (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:82585:18)
    at Object.getSymbolAtLocation (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:43783:21)
    at getReferencedFilesFromImportLiteral (/Users/octetstream/projects/qwik-test/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:120886:28)

Node.js v20.11.1
rendering chunks (19)... ELIFECYCLE  Command failed with exit code 1.
computing gzip size (0)...

 ELIFECYCLE  Command failed

It is important to mention that I need the .js extension, because I might need run parts of my code with ts-node. Case it: Mikro ORM cli support, which is relying on ts-node. TypeScript allows this extension and Node.js requires it for ESM paths. And this is why I set moduleResolution to node16 and not bundler.

@PatrickJS
Copy link
Member

PatrickJS commented May 7, 2024

oh yeah I see there were changing in tsconfig. I don't know enough about TypeScript to help with this maybe someone else can

ok it looks like we need to work on updating our "moduleResolution"

this seems like an optimizer issue. maybe @wmertens something with the optimizer to detect or ignore .js https://github.com/PatrickJS/qwik-typescript-js-extension-issue/tree/moduleResolution-bundler

image

@PatrickJS
Copy link
Member

yeah the quick fix might be a plugin to remap the .js to the correct files before qwik takes over

@PatrickJS PatrickJS changed the title [🐞] Support Typescript 4.7 "Node16" extension requirements [🐞] Support Typescript "moduleResolution": "node16" and "bundler" May 7, 2024
@PatrickJS PatrickJS added COMP: optimizer and removed STATUS-1: needs triage New issue which needs to be triaged STATUS-2: needs reproduction Issue lacks reproduction labels May 7, 2024
@ziimakc
Copy link
Author

ziimakc commented Jun 9, 2024

For anyone who want to use proper .js extension, install @rollup/plugin-typescript and use following config as example:

tsconfig.json

{
	"extends": "../../other/tsconfig.base.json",
	"compilerOptions": {
		"target": "ES2022",
		"module": "ES2022",
		"lib": ["es2022", "DOM", "WebWorker", "DOM.Iterable"],
		"jsx": "react-jsx",
		"jsxImportSource": "@builder.io/qwik",
		"moduleResolution": "Bundler",
		"esModuleInterop": true,
		"incremental": true,
		"outDir": "dist",
		"noEmit": true
	},
	"include": ["src", "./*.d.ts", "./*.config.ts"]
}

vite.config.ts:

import { qwikCity } from "@builder.io/qwik-city/vite";
import { qwikVite } from "@builder.io/qwik/optimizer";
import pluginTypescript from "@rollup/plugin-typescript";
import { defineConfig, type UserConfig } from "vite";

export default defineConfig(({ command, mode }): UserConfig => {
	const config: UserConfig = {
		plugins: [qwikCity(), qwikVite()],
		build: {
			rollupOptions: {
				plugins: [pluginTypescript()],
			},
		},
	};

	return config;
});

@wmertens
Copy link
Member

wmertens commented Jun 9, 2024

I don't understand, doesn't ts-node look at tsconfig? We recommend using module resolution Bundler.

@wmertens
Copy link
Member

@ziimakc is this still happening in 1.8.0?

@octet-stream
Copy link
Contributor

octet-stream commented Oct 3, 2024

I don't understand, doesn't ts-node look at tsconfig? We recommend using module resolution Bundler.

ts-node does not do anything with paths, because TS itself keeps paths untouched. It just keeps them as-is. But in order to ESM to work in Node.js, I need to use the .js specifier (see: https://nodejs.org/api/esm.html#mandatory-file-extensions), which Qwik (or rather Rollup, as far as I see from this conversation) cannot compile when targeting production build. I got the same error, with the same file (router-head).

I just tried 1.9.0 and the issue is still in the effect on a fresh project created via pnpm create qwik@latest.


Also, I encounter a new issue: TypeScript seem to have problems picking up JSX types (see the screenshot) when I use the same tsconfig.json as in the repo I linked above in one of my comments.

Details image

I see the same problem at least in v1.5.3 (which is the same version from the respo I linked above) and above. It seem to cover the whole InristicElements interface. But this probably have to be separated into different issue. Are JSX type from Qwik compatible with node16? I don't have this issue with Remix for example, so types from React are compatible.

@octet-stream
Copy link
Contributor

octet-stream commented Oct 3, 2024

Had to mention that as far as I remember, this problem only affects client-side of a bundle. Server-side code runs with .js specifier just fine, so I ended up just keeping moduleResolution set to Bundler and using the specifier only for server code until the issue is resolved (because, as I said, otherwise my code cannot be consumed by Node.js when I run it via ts-node). This is not consistent and I had to remember to use it on the server and keep track on which files will be referenced by Mikro ORM config. When moduleResolution is set to node16 tsc will require .js specifier at every absolute and relative path, so if I forgot one - my code won't compile and VSCode will also show me an error (and suggest .js when I write paths) which is to be expected.

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

No branches or pull requests

5 participants