components
components copied to clipboard
chore: Expose more component configuration in DOM metadata property
Description
This change extends the __awsuiMetadata__
property of 28 components to include information about their configured properties.
It builds on https://github.com/cloudscape-design/components/pull/1391, build failures are expected until the other PR is merged.
Related links, issue #, if available: n/a
How has this been tested?
Manual testing in the dev pages
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
- Changes include appropriate documentation updates.
-
Changes are backward-compatible if not indicated, see
CONTRIBUTING.md
. -
Changes do not include unsupported browser features, see
CONTRIBUTING.md
. - Changes were manually tested for accessibility, see accessibility guidelines.
Security
-
If the code handles URLs: all URLs are validated through the
checkSafeUrl
function.
Testing
- Changes are covered with new/existing unit tests?
- Changes are covered with new/existing integration tests?
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
The list of comments is not excessive, I only called repeating patterns once
The properties are not intended to be logged directly - an analytics library rather should use these to derive the values that it needs. For example, the Flashbar's items
property contains JSX, which cannot (usefully) be logged. Instead, an analytics library could e.g. log values like these:
{
infoMessagesCount: items.filter(item => item.type === "info").length,
errorMessagesCount: items.filter(item => item.type === "error").length,
serviceHasPersistentErrorStatus: items.some(item => item.type === "error" && !item.dismissible)
}
I did not want to preprocess data like this inside of our components, since that means a) we have to define all useful derived data, which may not cover future use cases, and b) it introduces a risk of becoming another stable API. The currently exposed properties are intentionally just the public component properties, since that API is fixed anyways (with the exception of the Tabs component, where we expose the actual value of the controllable activeTabId
property that might be undefined).
I see, I can approve this change, because it is internal, but there are some caveats making it harder to revisit if any issues occur.
- Changes propagate to production within weeks, so there will be moment when this property may contain different data depending on their dependency freshness
- These values may throw when passed into
JSON.stringify
, this may cause intermittent errors which will be hard to capture - Traversing some structures (side navigation items for example) may be tricky, because they are recursive. I believe we can simplify work for consumer end, preparing more digestable info (for example, annotating individual items)
Codecov Report
Patch coverage: 100.00%
and no project coverage change.
Comparison is base (
232149b
) 93.55% compared to head (6b78ec0
) 93.55%.
Additional details and impacted files
@@ Coverage Diff @@
## main #1398 +/- ##
=======================================
Coverage 93.55% 93.55%
=======================================
Files 627 627
Lines 16859 16859
Branches 5568 5568
=======================================
Hits 15772 15772
Misses 1014 1014
Partials 73 73
Files Changed | Coverage Δ | |
---|---|---|
src/select/index.tsx | 100.00% <ø> (ø) |
|
src/alert/index.tsx | 100.00% <100.00%> (ø) |
|
src/autosuggest/index.tsx | 100.00% <100.00%> (ø) |
|
src/breadcrumb-group/index.tsx | 100.00% <100.00%> (ø) |
|
src/button-dropdown/index.tsx | 100.00% <100.00%> (ø) |
|
src/cards/index.tsx | 93.90% <100.00%> (ø) |
|
src/checkbox/index.tsx | 100.00% <100.00%> (ø) |
|
src/date-picker/index.tsx | 96.61% <100.00%> (ø) |
|
src/date-range-picker/index.tsx | 97.11% <100.00%> (ø) |
|
src/icon/index.tsx | 100.00% <100.00%> (ø) |
|
... and 17 more |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.