-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
chore: setup utilities package #17280
chore: setup utilities package #17280
Conversation
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
I added the project IDs here and some comments about the Telemetry package versions. Everything else looks good.
Reviewing this, https://github.com/ibm-telemetry/telemetry-config-schema?tab=readme-ov-file#js-scope, it's unclear to me if we want to set allowedArgumentStringValues
in the JS functions configuration. I guess we can wait to see if we're capturing function arguments and if so, we can allowlist values like "long", "short", "narrow", etc. These are only for string arguments thought, and it looks like you're passing in locale and style in an object parameter, so given current Telemetry functionality, I don't think it's possible to track parameters here.
Also, I was going to suggest adding the jsx
config to the React package, but it doesn't have components, just hooks, and the js
scope with functions
will capture React hooks so we're good there.
…es-react Co-authored-by: Matt Rosno <[email protected]>
…into carbon-utilities
Will this actually work since these string values are passed as part of an object? |
@janhassel unfortunately no, as we only capture booleans, numbers, and allowed strings. We didn't have a secure way to capture object values. |
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.
looks good to me!
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, thanks again for doing this! 🎉
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17280 +/- ##
=======================================
Coverage 77.03% 77.03%
=======================================
Files 408 408
Lines 13979 13979
Branches 4339 4339
=======================================
Hits 10769 10769
Misses 3037 3037
Partials 173 173 ☔ View full report in Codecov by Sentry. |
fd2495b
Ref #16243
Changelog
New
@carbon/utilities
dateTimeFormat
module to it@carbon/utilities-react
useNoInteractiveChildren
from@carbon/react
as a starter