change: [M3-7232] Rename isPropValid and update codebase & documentation #9790
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abailly-akamai
changed the title
abailly-akamai
commented
Oct 12, 2023
| [ default export ] | ||
| [ exported function component definition ] | ||
| [ styles ] (possibly in their own file) | ||
| [ utility functions ] (possibly in their own file) |
There was a problem hiding this comment.
Some of these documentation changes also cover some of our newest conventions, not just the intent of the PR.
abailly-akamai
requested review from
carrillo-erik,
cpathipa,
jaalah-akamai and
mjac0bs
and removed request for
a team
jaalah-akamai
approved these changes
Oct 12, 2023
|
Just a heads up, I merged in the MUIv5 StackScripts pt1 PR, and it uses the old 'isPropValid' in this files:
Pt2 also uses isPropValid, but if your PR gets merged in first, I'll just resolve those conflicts on my end :D |
|
@coliu-akamai thx for the heads up! for pt2, may the fastest merge win |
mjac0bs
reviewed
Oct 12, 2023
Comment on lines
44
to
48
| /** | ||
| * It's often a good idea to move utilities to their own files as well, | ||
| * either in the `src/utilities` directory if meant to be portable and reusable, | ||
| * or in the feature's directory as a .utils.ts file. ex: `SayHello.utils.ts`. | ||
| * Isolation often makes them easier to test and reduce the main file size for better readability. |
abailly-akamai
force-pushed
the
M3-7232-prop-forwarding
branch
from
129d62f to
680042a
Compare
mjac0bs
approved these changes
Oct 12, 2023
bnussman-akamai
approved these changes
Oct 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description 📝
In our styled components, we have the ability to prevent the forwarding of custom props that could end up being passed as an invalid html attribute in the dom. To that effect we have a utility to use in
shouldForwardPropthat was namedisPropValidbut this name caused a bit of confusion since it made some assumption about the validity of the prop itself, while its intent is to only build an array of props to exclude from forwarding regardless of their semantic validity.Note: the
omittedPropsname has been discussed and vetted with @jaalah-akamaiChanges 🔄
isPropValidtoomittedPropsPreview 📷
There is no visual change brought by this PR, nor should there be any functionality change
How to test 🧪
Prerequisites
Verification steps
omittedPropsAs an Author I have considered 🤔
Check all that apply
Commit message and pull request title format standards
<commit type>: [JIRA-ticket-number] - <description>Commit Types:
feat: New feature for the user (not a part of the code, or ci, ...).fix: Bugfix for the user (not a fix to build something, ...).change: Modifying an existing visual UI instance. Such as a component or a feature.refactor: Restructuring existing code without changing its external behavior or visual UI. Typically to improve readability, maintainability, and performance.test: New tests or changes to existing tests. Does not change the production code.upcoming: A new feature that is in progress, not visible to users yet, and usually behind a feature flag.Example:
feat: [M3-1234] - Allow user to view their login history