Skip to content

Alternative fix for #3508 #3811

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

Merged
merged 7 commits into from
Oct 28, 2019
Merged

Alternative fix for #3508 #3811

merged 7 commits into from
Oct 28, 2019

Conversation

Rich-Harris
Copy link
Member

This is an alternative to #3809 (fixes #3508). It feels like a bit of a hack, but if we replace the array passed to the end of init with an object whose values are zeroes...

-init(this, options, instance, create_fragment, safe_not_equal, ["a", "b"]);
+init(this, options, instance, create_fragment, safe_not_equal, {a:0, b:0});

...then we can add renamed properties to the same object...

init(this, options, instance, create_fragment, safe_not_equal, {a:0, b:0, c:"d"});

...and handle aliased exports while adding slightly less code:

export function bind(component, name, callback) {
  if (name in component.$$.props) {
    name = component.$$.props[name] || name;
    component.$$.bound[name] = callback;
    callback(component.$$.ctx[name]);
  }
}

I don't feel great about it, but it works. @Conduitry thoughts?

@Rich-Harris Rich-Harris changed the title Gh 3508 alt Alternative fix for #3508 Oct 28, 2019
if (component.$$.props.indexOf(name) === -1) return;
component.$$.bound[name] = callback;
callback(component.$$.ctx[name]);
if (component.$$.props.hasOwnProperty(name)) {
Copy link
Member

@Conduitry Conduitry Oct 28, 2019

Choose a reason for hiding this comment

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

Linting doesn't like using Object.prototype.hasOwnProperty apparently - and probably for good reason, as I think this would get messed up with a prop called hasOwnProperty. What I had meant before was actually using Object.hasOwnProperty(component.$$.props, name).

Copy link
Member Author

Choose a reason for hiding this comment

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

wait, when did hasOwnProperty become a static method on Object? It works in Chrome, but does it work everywhere? I always used to do Object.prototype.hasOwnProperty.call(obj, prop)

Copy link
Member

@Conduitry Conduitry Oct 28, 2019

Choose a reason for hiding this comment

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

Apparently I'm going crazy. I just found this SO question, asked a mere three days ago. It sounds like Object.hasOwnProperty doesn't really exist, but, because of javascript mysteries, you can call it, but it doesn't actually do what I expected. Sorry about that! Object.prototype.hasOwnProperty.call(component.$$.props, name) sounds safest.

Edit: Or ({}).hasOwnProperty.call(...) to save bytes, I guess. I don't know whether that's a performance hit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out we're using hasOwnProperty somewhere already so I turned it into a has_prop helper

@Rich-Harris Rich-Harris merged commit 1273f97 into master Oct 28, 2019
@Rich-Harris Rich-Harris deleted the gh-3508-alt branch October 28, 2019 16:16
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.

Binding to an aliased property results in null value
2 participants