fix(declarations): bundle child_process type for portability #5165
Conversation
|
| Path | Error Count |
|---|---|
| src/dev-server/index.ts | 37 |
| src/mock-doc/serialize-node.ts | 36 |
| src/dev-server/server-process.ts | 32 |
| src/compiler/prerender/prerender-main.ts | 22 |
| src/testing/puppeteer/puppeteer-element.ts | 22 |
| src/runtime/client-hydrate.ts | 20 |
| src/screenshot/connector-base.ts | 19 |
| src/runtime/vdom/vdom-render.ts | 18 |
| src/compiler/config/test/validate-paths.spec.ts | 16 |
| src/dev-server/request-handler.ts | 15 |
| src/compiler/prerender/prerender-optimize.ts | 14 |
| src/compiler/sys/stencil-sys.ts | 14 |
| src/compiler/transpile/transpile-module.ts | 14 |
| src/runtime/vdom/vdom-annotations.ts | 14 |
| src/sys/node/node-sys.ts | 14 |
| src/compiler/prerender/prerender-queue.ts | 13 |
| src/compiler/sys/in-memory-fs.ts | 13 |
| src/runtime/connected-callback.ts | 13 |
| src/runtime/set-value.ts | 13 |
| src/compiler/output-targets/output-www.ts | 12 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2345 | 369 |
| TS2322 | 366 |
| TS18048 | 237 |
| TS18047 | 103 |
| TS2722 | 37 |
| TS2532 | 26 |
| TS2531 | 23 |
| TS2454 | 14 |
| TS2352 | 12 |
| TS2790 | 11 |
| TS2769 | 8 |
| TS2538 | 8 |
| TS2344 | 6 |
| TS2416 | 6 |
| TS2493 | 3 |
| TS18046 | 2 |
| TS2684 | 1 |
| TS2430 | 1 |
Unused exports report
There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
| File | Line | Identifier |
|---|---|---|
| src/runtime/bootstrap-lazy.ts | 21 | setNonce |
| src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
| src/testing/testing-utils.ts | 198 | withSilentWarn |
| src/utils/index.ts | 145 | CUSTOM |
| src/utils/index.ts | 269 | normalize |
| src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
| src/compiler/app-core/app-data.ts | 25 | BUILD |
| src/compiler/app-core/app-data.ts | 115 | Env |
| src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
| src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | satisfies |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | Record |
| src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
| src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
alicewriteswrongs
force-pushed
the
ap/fix-child-process-type-bug
branch
4 times, most recently
from
4ef8e4a
to
bd7ef7b
Compare
|
|
||
| import type { InMemoryFileSystem } from '../compiler/sys/in-memory-fs'; | ||
| import type { CPSerializable } from './child_process'; |
There was a problem hiding this comment.
this is a bit convoluted, I know, but the alternative is to keep importing directly from child_process in this file and then run dts-bundle-generator on the whole thing (build/declarations/stencil-private.d.ts). In some rough testing this took the size of internal/stencil-private.d.ts from ~69k to ~160k, which I thought was too much. So instead we stand up a module that just re-exports one type from child_process, and we can then bundle that with dts-bundle-generator and ship it right next to stencil-private.d.ts so it can import the inlined type from it, making stencil-private.d.ts portable.
| }, | ||
| }, | ||
| ]).join('\n'); | ||
| await fs.writeFile(childProcessDestPath, childProcessDts); |
There was a problem hiding this comment.
If someone wonders what this results to: it creates a file in internal/child_process.d.ts with this content:
type Serializable = string | object | number | boolean | bigint;
export {
Serializable as CPSerializable,
};
export {};
|
For testing this overall, i.e. putting a regression test in place, I thought that the component starter tests should catch problems of this type but evidently not. I'll look into how we might modify them to do that a bit |
|
I realized why most users wouldn't have run into this issue. If you start a Stencil project doing the following: npm init stencil@latest component test-node-type-failureand then do npm i
npm run buildyou won't see the error about missing types, which makes sense since if you check However if your project is only depending on So I'm going to add a smoke test which just checks that the project runs without errors when it is the only dependency installed in a project |
|
(I'll add that in a separate PR so we can first confirm that the test fails, merge this, rebase, and confirm it then passes) |
|
@alicewriteswrongs afaik another way to potentially test this is to define |
This adds a smoke test that checks that a project with only `@stencil/core` installed will build a simple component without errors. When working on #5165 it became clear we should have a regression test in place to ensure we know right away if a change makes a 'bare' stencil project (i.e. with only `@stencil/core` installed) is going to error out.
|
Oh true, well I already put together #5170 which I kind of think we'll want anyway (it covers a broader range of potential issues than just types missing) |
alicewriteswrongs
force-pushed
the
ap/fix-child-process-type-bug
branch
from
bd7ef7b
to
0027481
Compare
|
@tanner-reits @christian-bromann just updated that import (no idea why it was like that in the first place!) |
We need to make sure our type declarations don't depend on types we don't bundle! fixes #5162 STENCIL-1077
alicewriteswrongs
force-pushed
the
ap/fix-child-process-type-bug
branch
from
0027481
to
c1fb2b1
Compare
We need to make sure our type declarations don't depend on types we don't bundle!
fixes #5162
STENCIL-1077
What is the current behavior?
GitHub Issue Number: N/A
What is the new behavior?
Does this introduce a breaking change?
Testing
There's a reproduction case on #5162. You could test that this fixes the issue by building and packing this locally, installing it in that reproduction case, and confirming that you don't get a type error when building.
Other information