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

Ylem Component does not re-render when props changes #193

Open
ivospinheiro opened this issue Oct 28, 2019 · 5 comments
Open

Ylem Component does not re-render when props changes #193

ivospinheiro opened this issue Oct 28, 2019 · 5 comments

Comments

@ivospinheiro
Copy link

Components implemented based on Ylem.Component are not updating the when props changes.

Here is a small example based on the Ylem sample CodeSandBox that reproduces the issue:
https://codesandbox.io/s/ylem-paginated-grid-demo-gnn6d?fontsize=14

@ivospinheiro
Copy link
Author

I've debugged the code and it seems to be related with the following lines of code:

shouldComponentUpdate(props, state) {
if (!this.observer
|| (super.shouldComponentUpdate && !super.shouldComponentUpdate(props, state))
|| !this.shouldUpdate) {
return false;
}
this.shouldUpdate = false;
return true;

React.Component does not provide a default implementation for shouldComponentUpdate.

@BigAB BigAB self-assigned this Oct 28, 2019
@BigAB
Copy link
Contributor

BigAB commented Oct 28, 2019

Interesting. I wonder if that ever worked or did something change.

I'll try to look at this when I can.

@christopherjbaker
Copy link
Contributor

I believe that super.shouldComponentUpdate is leftover from a slightly different implementation; but since it is always undefined, it won't be the cause of any issues.

In our tests, we primarily bind props to the store to manage updates. (That's not to say your example shouldn't work, only identifying why it was missed in our tests).
https://github.com/bitovi/ylem/blob/master/test/component.js#L82-L98

@christopherjbaker
Copy link
Contributor

I also wanted to link the CodeSandbox that Ivo had created: https://codesandbox.io/s/ylem-props-issue-lmscf

@frank-dspeed
Copy link

i think its related to can-observer.js inside lib its doing

		this.onDependencyChange = (newVal, oldVal) => {
			this.dependencyChange(newVal, oldVal);
		};

but the function depenedencyChange has no arguments :)

	dependencyChange() {
		queues.deriveQueue.enqueue(this.onUpdate, this, [], { priority: this.order });
	}

@BigAB BigAB removed their assignment Jun 23, 2020
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

No branches or pull requests

4 participants