Skip to content
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

withFilters: Filter is not applied #8567

Closed
mmtr opened this issue Aug 5, 2018 · 8 comments
Closed

withFilters: Filter is not applied #8567

mmtr opened this issue Aug 5, 2018 · 8 comments
Assignees
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@mmtr
Copy link
Contributor

mmtr commented Aug 5, 2018

Describe the bug
When rendering a component using the withFilters higher order component outside Gutenberg, the filter is not applied.

To Reproduce
Execute the code below in a new React project after installing @wordpress/components and @wordpress/hooks.

import { withFilters } from '@wordpress/components';
import { addFilter } from '@wordpress/hooks';

const ComposedComponent = () => <div>Composed component</div>;

addFilter(
	'MyHookName',
	'example/filtered-component',
	( FilteredComponent ) => () => (
		<div>
			<FilteredComponent />
			<ComposedComponent />
		</div>
	)
);

const MyComponentWithFilters = withFilters( 'MyHookName' )( 
	() => <div>My component</div> 
);

ReactDOM.render(
	<MyComponentWithFilters />,
	document.getElementById( 'root' )
);

Expected behavior
"Composed component" should appear below "My component".

Screenshots
screen shot 2018-08-05 at 23 53 56

Desktop:

  • OS: macOS High Sierra
  • Browser: Chrome
  • Version: 67

Additional context
Issue found while working on #8338 and Automattic/wp-calypso#26367

@danielbachhuber danielbachhuber added [Type] Bug An existing feature does not function as intended Framework Issues related to broader framework topics, especially as it relates to javascript labels Aug 6, 2018
@gziolo gziolo self-assigned this Aug 6, 2018
@gziolo gziolo added the [Package] Components /packages/components label Aug 6, 2018
@gziolo
Copy link
Member

gziolo commented Aug 6, 2018

I can't reproduce inside Gutenberg:

screen shot 2018-08-06 at 15 24 18

Can you try again outside of LiveReact context and confirm?

@mmtr
Copy link
Contributor Author

mmtr commented Aug 6, 2018

you're right! react-live is causing the issue. I have tested it in a clean project and is working nicely:

I'll try to figure out how to solve the issue with react-live.

sorry for false alarm!

@mmtr mmtr closed this as completed Aug 6, 2018
@mmtr
Copy link
Contributor Author

mmtr commented Aug 11, 2018

@mmtr mmtr reopened this Aug 11, 2018
@gziolo
Copy link
Member

gziolo commented Aug 11, 2018

Don’t you encounter a similar issue as with data package where 2 instances of the same library with different versions are loaded?

@mmtr
Copy link
Contributor Author

mmtr commented Aug 11, 2018

I think you're confusing with someone else. I have not played around yet with the data package.

@gziolo
Copy link
Member

gziolo commented Aug 13, 2018

It was a question, see a related discussion where two different versions of the same library were loaded causing issues with internal state: Automattic/wp-calypso#26438 (comment)

@jsnajdr can you help to debug this one?

@jsnajdr
Copy link
Member

jsnajdr commented Aug 13, 2018

Yes, it looks like the @wordpress/hooks module is duplicated and because it contains a filter registry, the addFilter and applyFilters function work with different registries.

On my local wp-calypso, I have 5 copies:

$ find . -path "*/@wordpress/hooks"
./node_modules/@wordpress/blocks/node_modules/@wordpress/hooks
./node_modules/@wordpress/api-fetch/node_modules/@wordpress/hooks
./node_modules/@wordpress/components/node_modules/@wordpress/hooks
./node_modules/@wordpress/hooks
./node_modules/@wordpress/editor/node_modules/@wordpress/hooks

The solution is to regenerate the npm-shrinkwrap.json file with npm run update-deps. That forces NPM to deduplicate the modules. It's also important that @wordpress/hooks is a top level dependency of Calypso, but it already is.

@gziolo
Copy link
Member

gziolo commented Aug 13, 2018

@jsnajdr, thanks for double checking this one. I'm closing this issue in here, because it isn't an issue with hooks itself. We will try to come up with an action item to attack this general issue of consuming multiple @wordpress/* packages at once tomorrow during Core JS meeting.
See: https://docs.google.com/document/d/1KNa4xxktVskFV86SVMbdTWx3bINrLikBnJpntBZw1DU/edit.

@gziolo gziolo closed this as completed Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants