-
Notifications
You must be signed in to change notification settings - Fork 789
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
fix(compiler): make sure typesDir exist before writing to it #5109
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/style/test/optimize-css.spec.ts | 23 |
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts | 22 |
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 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2345 | 399 |
TS2322 | 374 |
TS18048 | 286 |
TS18047 | 101 |
TS2722 | 37 |
TS2532 | 30 |
TS2531 | 23 |
TS2454 | 14 |
TS2352 | 13 |
TS2790 | 10 |
TS2769 | 8 |
TS2538 | 8 |
TS2416 | 6 |
TS2344 | 5 |
TS2493 | 3 |
TS2488 | 2 |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this out and it works! Ran the script a few times and never had an instance where it didn't generate the types.
I'm not sure what the consequences of removing immediateWrite
would be. The call to generateAppTypes
in runTsProgram
is only responsible for generating the src/components.d.ts
file. Not sure if that matters 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working as intended - blocking on a small ask only because it's technically a breaking change
b0e8783
to
f128837
Compare
Released in |
fixes #5091 / STENCIL-1024
What is the current behavior?
Given the following Stencil config:
running a Stencil build causes the types directory
/dist/types
not being generated.My investigations were as follows:
{ type: 'dist-custom-elements' }
entry inoutputTargets
seems to cause the type generation to run twice with the same configurationsgenerateTypes
to be called twice as well as the command to write/dist/types/components.d.ts
with the optionimmediateWrite: true
fs.writeFile
is called in/src/sys/node/node-sys.ts#L612-L614
GitHub Issue Number: #5091
What is the new behavior?
As seen in the change-set it turns out that we didn't handle the error provided by the
sys.writeFile
. Essentially the call to create the file failed without handling its error. Further investigations have shown that the directory for the file didn't exist even though we were callingensureDir
before. The race condition originates from the question how fast other output items were able to generate their types (I believe).My suggestion to resolve this issue is to use the
recursive
flag for creating the directory when we are not working within memory. I am afraid callingmkdir
multiple times, especially ongraceful-fs
rather than the Node.js package must have caused weird side effects in the event loop. During the investigations I literally was able to resolve it by adding a simpleconsole.log
.An alternative solution would be to remove the
immediateWrite: true
option and have it write to the filesystem when everything file operation is being flushed which is likely a point in time the directory has been created already. However since we are callinggenerateAppTypes
also inrunTsProgram
I assume it is important to keep this option so rewrites are happening on watch mode!?Does this introduce a breaking change?
Testing
See Jira ticket, apply the patch provided and reproduce the issue.
To test this change:
npm run build
"@stencil/core": "file:/Users/christian.bromann/Sites/Ionic/projects/stencil"
./loop.sh
and see that thetypes
dir is created every timeOther information