ionic-team
fix(renderer): fix missing slot ref callback handling
fixes one of the issues raised in #5335
adds a new internal variable which is stored during vnode rendering to reuse it within the relocatedNodes logic
What is the current behavior?
Slot element's ref attribute is not handled correctly.
GitHub Issue Number: #5335
What is the new behavior?
Fixed
Documentation
Does this introduce a breaking change?
- [ ] Yes
- [x] No
Testing
Other information
--strictNullChecks error report
Typechecking with --strictNullChecks resulted in 1145 errors on this branch.
That's the same number of errors on main, so at least we're not creating new ones!
reports and statistics
Our most error-prone files
| Path | Error Count |
|---|---|
| src/dev-server/index.ts | 37 |
| 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 | 17 |
| 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/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 |
| src/compiler/transformers/test/parse-vdom.spec.ts | 12 |
| src/compiler/transformers/transform-utils.ts | 12 |
| src/mock-doc/test/attribute.spec.ts | 12 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2322 | 361 |
| TS2345 | 349 |
| TS18048 | 207 |
| TS18047 | 82 |
| TS2722 | 37 |
| TS2532 | 24 |
| TS2531 | 21 |
| TS2454 | 14 |
| TS2790 | 11 |
| TS2352 | 10 |
| TS2769 | 8 |
| TS2538 | 8 |
| 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 |
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
PR built and packed!
Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8246043498/artifacts/1317838396
If your browser saves files to ~/Downloads you can install it like so:
unzip -d ~/Downloads ~/Downloads/stencil-core-4.12.6-dev.1710233775.a171a02.tgz.zip && npm install ~/Downloads/stencil-core-4.12.6-dev.1710233775.a171a02.tgz
Hi @christian-bromann thanks for the detailed explanation. I just updated the tests.
However, even in the main branch 3 of 156 test cases are failing. It would be worth checking. I added my test and it seems it works. I know a bit about the TDD. So initially, I implemented the failing test case and then corrected it. So I am pretty sure it is passing 😃
One more question for future contributions. I don't have that much experience with karma, unfortunately. And as of now, I have done the following steps to have a working testing environment. Please confirm that the steps are ok and please describe a way to make this operation more easier such as starting it in watch mode or sth like that.
cd test/karma
npm i
# npm run karma # failed
npm run karma.prod
However, even in the main branch 3 of 156 test cases are failing. It would be worth checking.
Make sure you have the project compile (npm run build) and run npm run test.karma.prod. This also ensures that all example projects are compiled with the Stencil build you have.
Please confirm that the steps are ok and please describe a way to make this operation more easier such as starting it in watch mode or sth like that.
As mentioned above the best way to start the e2e tests are by running npm run test.karma.prod. We are currently in the process to migrate over to WebdriverIO for browser based unit tests. In fact we start with this work next sprint (next week). We will make sure to document the process accordingly but I am sure it will become simpler moving on 😉
@yigityuce Can you run npm run format when you get a chance on this branch? It looks like that CI check is currently failing
@rwaskiewicz
Can you run npm run format when you get a chance on this branch? It looks like that CI check is currently failing
done 🚀