-
Notifications
You must be signed in to change notification settings - Fork 795
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(docs): merge together style docs from multiple CSS files #5653
Conversation
PR built and packed!Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8691594249/artifacts/1414854856 If your browser saves files to
|
|
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 | 344 |
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 |
028ecc9
to
f2a22dc
Compare
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.
LGTM!
This fixes a bug where although multiple stylesheets were being processed by the `ext-transforms-plugin` we were not properly merging the style documentation for all of those stylesheets, leading to a race condition where whichever file was processed last would set the style docs for the whole component. Not good! To fix the issue we just need to merge the style docs for the various stylesheets together (de-duping on the `name` property). This also adds a test project in `test/docs-readme` which exercises this functionality. STENCIL-1271
112a00b
to
6a486da
Compare
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.
👍
Released in Stencil ♨️ v4.17.0 |
This fixes a bug where although multiple stylesheets were being processed by the
ext-transforms-plugin
we were not properly merging the style documentation for all of those stylesheets, leading to a race condition where whichever file was processed last would set the style docs for the whole component. Not good!To fix the issue we just need to merge the style docs for the various stylesheets together (de-duping on the
name
property).What is the current behavior?
The last stylesheet to be processed 'wins' and defines the
styleDocs
property on theComponentCompilerMetadata
for a given component.What is the new behavior?
As new
StyleDocs
come in we merge them into one array, to ensure we're collecting them all.Does this introduce a breaking change?
Testing
This functionality is a bit difficult to unit-test, so I stood up a test project similar to the one we already have for testing the
docs-json
output target. It includes a reproduction of the issue, where a component has two css files associated with it instyleUrls
which both have CSS props documented in them. Both of these should show up in the README for the file.