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): throw proper error if component is loaded with invalid runtime #5675
base: main
Are you sure you want to change the base?
Conversation
|
Path | Error Count |
---|---|
src/dev-server/index.ts | 37 |
src/dev-server/server-process.ts | 32 |
src/compiler/prerender/prerender-main.ts | 22 |
src/runtime/client-hydrate.ts | 20 |
src/testing/puppeteer/puppeteer-element.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/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/compiler/transpile/transpile-module.ts | 12 |
src/mock-doc/test/attribute.spec.ts | 12 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2322 | 361 |
TS2345 | 343 |
TS18048 | 204 |
TS18047 | 82 |
TS2722 | 37 |
TS2532 | 24 |
TS2531 | 21 |
TS2454 | 14 |
TS2790 | 11 |
TS2352 | 9 |
TS2769 | 8 |
TS2538 | 8 |
TS2416 | 7 |
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 |
PR built and packed!Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8789074694/artifacts/1436726702 If your browser saves files to
|
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.
just had a few questions / comments after taking an initial look
/** | ||
* Fetching logs like this only works in Chromium. Once WebdriverIO v9 is released there | ||
* will be easier primitives to fetch logs in other browsers as well. | ||
*/ |
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.
we might want to create a JIRA ticket for upgrading to WebdriverIO v9 and then leave a TODO here to remove the browser.isChromium
check
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.
created STENCIL-1291
Thanks @alicewriteswrongs for the feedback, applied suggested changes. |
sorry for the delay on the review on this one! |
What is the current behavior?
Given someone imports a component compiled with
dist-custom-element
into a Stencil project that uses thedist
output target. In the case that the DCE component shares the same runtime as thedist
elements we will run into a runtime error.GitHub Issue Number: #5596
What is the new behavior?
This patch proposes to throw a more meaningful error in these situations.
I wonder if there is a better solution where we detect this at compile time. One idea would be to make the DCE component import an object from the runtime library that is only exported if the runtime is dedicated to DCE components.
Documentation
The
externalRuntime
property is documented however, we don't particularly mention this use case.Does this introduce a breaking change?
If the instance is
undefined
in this case an error would be thrown anyway incallRender
as we passinstance
along asundefined
.Testing
I added an e2e test for this scenario.
Other information