Skip to content

Conversation

@icatalina
Copy link
Contributor

@icatalina icatalina commented Oct 28, 2025

Jira: B2B-3845

What/Why?

Fix warning about invalid props being passed to html components.

Avoid passing invalid props through styled components directly to HTML components, doing so will trigger a WARNING.

We do that using a filter for @emotion/styled

Rollout/Rollback

Revert

Testing

Before:

image

After:

image

These warnings show up in the console in the browser and in some tests sometimes.

@icatalina icatalina requested a review from a team as a code owner October 28, 2025 23:10
alignItems,
textAlignLocation,
}: FlexItemProps) => ({
const ignoreDollarProps = (prop: string): boolean => isPropValid(prop) && !prop.startsWith('$');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignoring $ props is the pattern that StyledComponents follows by default.


it('should keep checkbox selection even after the product Qty update', async () => {
vitest.mocked(useParams).mockReturnValue({ id: '272989' });
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
Copy link
Contributor Author

@icatalina icatalina Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these spies where there to silence these issues, no longer required.

…d to html components

Avoid passing invalid props through styled components directly to HTML
components, doing so will trigger a WARNING.
@icatalina icatalina force-pushed the icatalina/B2B-3845/fix-invalid-props branch 2 times, most recently from f7243eb to 6d96574 Compare October 28, 2025 23:42
await waitForElementToBeRemoved(() => screen.queryByText(/loading/i));

const rowOfLovelyBoots = screen.getByRole('row', { name: /Lovely boots/ });
within(rowOfLovelyBoots).getByRole('checkbox').click();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the .blur() underneath are throwing a "Not wrapped in an act" warning, replacing for userEvent.click and userEvent.tab which correctly wrap things in an act.

@icatalina icatalina force-pushed the icatalina/B2B-3845/fix-invalid-props branch from 6d96574 to f7243eb Compare October 31, 2025 12:54
Copy link
Contributor

@bc-victor bc-victor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like tests are still failing even after rerunning, also one very minor comment

describe('Add to quote', () => {
it('add shopping list to draft quote', async () => {
vitest.mocked(useParams).mockReturnValue({ id: '272989' });
// const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets delete this line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, def. sorry...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants