-
Notifications
You must be signed in to change notification settings - Fork 785
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(runtime): support "capture" style events #4968
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/build/build-stats.ts | 27 |
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts | 25 |
src/compiler/style/test/optimize-css.spec.ts | 23 |
src/testing/puppeteer/puppeteer-element.ts | 23 |
src/compiler/prerender/prerender-main.ts | 22 |
src/runtime/vdom/vdom-render.ts | 20 |
src/runtime/client-hydrate.ts | 19 |
src/screenshot/connector-base.ts | 19 |
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/build/build-finish.ts | 13 |
src/compiler/prerender/prerender-queue.ts | 13 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2345 | 418 |
TS2322 | 398 |
TS18048 | 310 |
TS18047 | 100 |
TS2722 | 38 |
TS2532 | 34 |
TS2531 | 23 |
TS2454 | 14 |
TS2352 | 13 |
TS2769 | 10 |
TS2790 | 10 |
TS2538 | 8 |
TS2344 | 5 |
TS2416 | 4 |
TS2493 | 3 |
TS18046 | 2 |
TS2684 | 1 |
TS2488 | 1 |
TS2430 | 1 |
Unused exports report
There are 12 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/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 | 62 | satisfies |
src/compiler/types/validate-primary-package-output-target.ts | 62 | 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.
Two minor questions/asks
src/runtime/vdom/set-accessor.ts
Outdated
let capture = false; | ||
if (memberName.endsWith(CAPTURE_EVENT_SUFFIX)) { | ||
capture = true; |
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.
Minor, couldn't we avoid the double assignment here of capture
by using the conditional directly since we're always calling it?
let capture = false; | |
if (memberName.endsWith(CAPTURE_EVENT_SUFFIX)) { | |
capture = true; | |
const capture = memberName.endsWith(CAPTURE_EVENT_SUFFIX); | |
if (capture) { |
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.
Made that change, also just removed the conditional that would follow so we always just make the call to replace
, just won't do anything if there's nothing to replace (obviously).
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 locally and it works great!
What is the current behavior?
Our element interfaces include capture type events (like
onClickCapture
,onInputCapture
, etc.), but our runtime does not correctly bind the event listeners for these types. So, currently, nothing happens when using these attributes since they were bound like this:but should be bound like this:
Fixes: #4955
What is the new behavior?
The runtime has been updated to correctly bind the listeners for capture events based on the "Capture" suffix in the event attribute
Does this introduce a breaking change?
Testing
Manual testing
onClickCapture
to any element in a Stencil component starter that just logs out the event. Notice nothing gets logged clicking the element.Automated testing
setAccessor
to ensure the call to bind the event listeners is getting the correct argsOther information