fix(node): fix type errors #1642
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
| @@ -113,7 +113,7 @@ function __napi_rs_initialize_modules(__napiInstance) { | |||
| __napiInstance.exports['__napi_register__BindingRenderedModule_struct_79']?.() | |||
| __napiInstance.exports['__napi_register__AliasItem_struct_80']?.() | |||
| __napiInstance.exports['__napi_register__BindingSourcemap_struct_81']?.() | |||
| __napiInstance.exports['__napi_register__BindingJSONSourcemap_struct_82']?.() | |||
| __napiInstance.exports['__napi_register__BindingJsonSourcemap_struct_82']?.() | |||
There was a problem hiding this comment.
I have a question about wasi output. It looks like there are two similar looking code here under packages/rolldown/src and also at the package root packages/rolldown.
When I build locally just build and just build wasi, the ones in packages/rolldown/src updated but package/rolldown hasn't changed. Could I be missing some local setup?
There was a problem hiding this comment.
I'm confused too. You could just leave it there for now.
There was a problem hiding this comment.
Is this only for type check on dist? Does the situation that compiled dts is wrong happen a lot?
There was a problem hiding this comment.
Does the situation that compiled dts is wrong happen a lot?
I don't think it usually happens since if it's emitted by tsc or rollup dts, then they are supposed to be correct. (actually I'm not sure why current build-types script's tsc is not able to spot typo like xxxJSON and xxxJson).
As in the comment, I mostly copied the one from https://github.com/vitejs/vite/blob/main/packages/vite/tsconfig.check.json, but it might be overkill to do the same on rolldown.
|
The pull request probably not enough to fix all issues, I pull your branch and link to local, there are still 17 type errors exists: ../../../rolldown/packages/rolldown/dist/types/utils/asset-source.d.ts:1:36 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import { BindingAssetSource } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/options/normalized-input-options.d.ts:1:104 - error TS2307: Cannot find module '../rollup' or its corresponding type declarations.
1 import type { LogLevelOption, RollupLog, NormalizedInputOptions as RollupNormalizedInputOptions } from '../rollup';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/types/sourcemap.d.ts:1:34 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import { BindingSourcemap } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/plugin-context-data.d.ts:1:38 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import { BindingPluginContext } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/plugin-context.d.ts:1:51 - error TS2307: Cannot find module '../rollup' or its corresponding type declarations.
1 import type { RollupError, LoggingFunction } from '../rollup';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/plugin-context.d.ts:2:43 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
2 import type { BindingPluginContext } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/transfrom-plugin-context.d.ts:1:74 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import type { BindingPluginContext, BindingTransformPluginContext } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/transfrom-plugin-context.d.ts:2:34 - error TS2307: Cannot find module '../rollup' or its corresponding type declarations.
2 import type { RollupError } from '../rollup';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/options/output-options.d.ts:1:36 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import type { RenderedChunk } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/options/normalized-output-options.d.ts:1:78 - error TS2307: Cannot find module '../rollup' or its corresponding type declarations.
1 import type { SourcemapIgnoreListOption, SourcemapPathTransformOption } from '../rollup';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/options/normalized-output-options.d.ts:4:36 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
4 import type { RenderedChunk } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/log/logger.d.ts:1:63 - error TS2307: Cannot find module '../rollup' or its corresponding type declarations.
1 import type { LoggingFunction, LogHandler, RollupError } from '../rollup';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/builtin-plugin.d.ts:1:64 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import { BindingBuiltinPlugin, BindingBuiltinPluginName } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/index.d.ts:1:70 - error TS2307: Cannot find module '../binding' or its corresponding type declarations.
1 import type { BindingHookResolveIdExtraOptions, RenderedChunk } from '../binding';
~~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/plugin/index.d.ts:11:32 - error TS2307: Cannot find module '../rollup' or its corresponding type declarations.
11 import type { RollupLog } from '../rollup';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/index.d.ts:15:42 - error TS2307: Cannot find module './binding' or its corresponding type declarations.
15 import { RenderedChunk, transform } from './binding';
~~~~~~~~~~~
../../../rolldown/packages/rolldown/dist/types/index.d.ts:25:62 - error TS2307: Cannot find module './rollup' or its corresponding type declarations.
25 export type { RollupError, RollupLog, LoggingFunction } from './rollup';
~~~~~~~~~~
Found 17 errors.
⋊> ~/D/r/i/r/rolldown-typescript-error on main ⨯ 17:34:22
|
I'm retest at vite build at my local, not found issue, i'm worry for me... But i think it should be fixed and type check for dist. Here has some complex for bundling d.ts, let us waiting some days to fix it. |
|
Maybe it is related to microsoft/TypeScript#5112 |
|
This simple patch seems worked, diff --git a/packages/rolldown/package.json b/packages/rolldown/package.json
index b80d329b..551a63b2 100644
--- a/packages/rolldown/package.json
+++ b/packages/rolldown/package.json
@@ -58,6 +58,7 @@
"bak_build-node": "unbuild",
"build-node": "node ../../node_modules/npm-rolldown/bin/cli.js -c ./rolldown.config.mjs",
"build-types": "tsc -p ./tsconfig.dts.json",
+ "post:build-types": "cp src/*.d.ts ./dist/types/",
"build-native:debug": "run-s build-binding build-types build-node",
"build-native:release": "run-s build-binding:release build-types build-node",
"build-wasi:debug": "run-s build-binding:wasi build-node", |
|
@IWANABETHATGUY Could you push it and merge the pr. I think it's ok for me. |
Better add to |
|
CI failed. |
Moving the copy operation to |
When running rm -rf packages/rolldown/dist
just build
ls packages/rolldown/dist/types/binding.d.ts |
Then ignore me, just fix the CI, it is good to merge. |
Description
This PR renames rust side struct to match js naming
BindingJsonSourcemapto workaround napi's inconsistent dts emit. I also addedbuild-types-checkto verify type check passes for its package types exports (similar to how vite does).I'm not sure changing rust side naming is ideal. Another workaround is to prevent napi renaming by
napi_derive::napi(object, js_name = "BindingJSONSourcemap")). I don't think I can figure the fix on napi side, so I thought it's worth suggesting a quick fix here. Please let me know what you think. Thanks!