Skip to content

Bug: Fixing null/undefined theme argument for the WordPressBlocksProvider #2013

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.

Sign up for GitHub

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

Merged
josephfusco merged 14 commits into from
Jan 8, 2025
Merged

Bug: Fixing null/undefined theme argument for the WordPressBlocksProvider #2013

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fb7f89c
Fixes #1986 and allows for the theme argument to be null for the Word…
colinmurphy Dec 20, 2024
4514aed
Fixed typo
colinmurphy Dec 20, 2024
1e6fd19
Fixing lint issue if theme context is undefined.
colinmurphy Dec 20, 2024
24f4d0e
Fixed conditions to allow null for the theme argument
colinmurphy Dec 20, 2024
54cfbaf
Fixed logic
colinmurphy Dec 20, 2024
d7146e3
Fixed lint issues
colinmurphy Dec 20, 2024
73f8a2f
Fixed typo after running eslint and it changed WordPressThemeContext
colinmurphy Dec 23, 2024
0493012
Returning theme context as this isn't required by the WordPressBlockP…
colinmurphy Dec 24, 2024
b94fd14
Set return type as BlocksTheme
colinmurphy Dec 24, 2024
0d65fa3
Removed test assertion to check if theme is undefined
colinmurphy Dec 24, 2024
1d1f608
Updating to use a default value for theme instead in order to prevent…
colinmurphy Jan 3, 2025
c53812b
Removed testing code
colinmurphy Jan 3, 2025
c6fcb91
Reverted test deletion.
colinmurphy Jan 8, 2025
9059522
Addec changeset
colinmurphy Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/blocks/src/components/WordPressBlocksProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function WordPressBlocksProvider(props: {
config: WordPressBlocksProviderConfig;
}) {
const { children, config } = props;
const { blocks, theme } = config;
const { blocks, theme = {} } = config;

return (
<WordPressBlocksContext.Provider value={blocks}>
Expand Down
9 changes: 1 addition & 8 deletions packages/blocks/tests/components/WordPressBlocksProvider.test.tsx
View file
Open in desktop
Copy link
Member

@moonmeister moonmeister Jan 6, 2025

Choose a reason for hiding this comment

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

@colinmurphy I'm guessing we need to leave these tests in now that we've changed the fix, yes?

Copy link
Member Author

@colinmurphy colinmurphy Jan 7, 2025

Choose a reason for hiding this comment

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

@moonmeister We probably don't. I was leaving this for reference if someone wanted to use the theme argument but I removed the failing test for undefined.

Copy link
Member Author

@colinmurphy colinmurphy Jan 8, 2025

Choose a reason for hiding this comment

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

@moonmeister Tests reverted back. Thanks for pointing this out.

Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,6 @@ import type { BlocksTheme } from '../../src/types/theme';
import { renderHook } from '@testing-library/react'; // Import from @testing-library/react

describe('useBlocksTheme', () => {
it('Throws an error if not used within WordPressBlocksProvider', () => {
// Assert that renderHook throws an error when used outside of WordPressBlocksProvider
expect(() => {
renderHook(() => useBlocksTheme());
}).toThrow('useBlocksTheme hook was called outside of context, make sure your app is wrapped with WordPressBlocksProvider');
});

it('returns the passed in theme from WordPressBlocksProvider', () => {
// Wrapping component to provide context
const wrapper = ({ children }: PropsWithChildren<{}>) => {
Expand Down Expand Up @@ -46,4 +39,4 @@ describe('useBlocksTheme', () => {
// Check the correct theme is returned
expect(theme?.colors?.palette).toStrictEqual({ primary: 'black' });
});
});
});
Loading