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.

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
Rich-Harris merged 7 commits into from
Oct 28, 2019
Merged

Alternative fix for #3508 #3811

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2b71798
fix binding to aliased props (#3508)
Conduitry Oct 27, 2019
8797df9
add test
Conduitry Oct 27, 2019
dbdcb60
update changelog
Conduitry Oct 27, 2019
1c7d61e
Merge branch 'gh-3508' of https://github.com/Conduitry/svelte into Co…
Rich-Harris Oct 27, 2019
47a9811
alternative fix for #3508
Rich-Harris Oct 28, 2019
de2b1a6
use hasOwnProperty, simplify prop object construction
Rich-Harris Oct 28, 2019
9d11827
add has_prop helper
Rich-Harris Oct 28, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Also:
* Fix edge cases in matching selectors against elements ([#1710](https://github.com/sveltejs/svelte/issues/1710))
* Fix several bugs related to interaction of `{...spread}` attributes with other features ([#2721](https://github.com/sveltejs/svelte/issues/2721), [#2916](https://github.com/sveltejs/svelte/issues/2916), [#3421](https://github.com/sveltejs/svelte/issues/3421), [#3681](https://github.com/sveltejs/svelte/issues/3681), [#3764](https://github.com/sveltejs/svelte/issues/3764), [#3790](https://github.com/sveltejs/svelte/issues/3790))
* Allow exiting a reactive block early with `break $` ([#2828](https://github.com/sveltejs/svelte/issues/2828))
* Fix binding to props that have been renamed with `export { ... as ... }` ([#3508](https://github.com/sveltejs/svelte/issues/3508))
* Fix application of style scoping class in cases of ambiguity ([#3544](https://github.com/sveltejs/svelte/issues/3544))
* Check attributes have changed before setting them to avoid image flicker ([#3579](https://github.com/sveltejs/svelte/pull/3579))
* Fix generating malformed code for `{@debug}` tags with no dependencies ([#3588](https://github.com/sveltejs/svelte/issues/3588))
Expand Down
13 changes: 5 additions & 8 deletions src/compiler/compile/render_dom/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import add_to_set from '../utils/add_to_set';
import { extract_names } from '../utils/scope';
import { invalidate } from '../utils/invalidate';
import Block from './Block';
import { ClassDeclaration, FunctionExpression, Node, Statement } from 'estree';
import { ClassDeclaration, FunctionExpression, Node, Statement, ObjectExpression } from 'estree';

export default function dom(
component: Component,
Expand Down Expand Up @@ -223,7 +223,7 @@ export default function dom(

component.rewrite_props(({ name, reassigned, export_name }) => {
const value = `$${name}`;

const insert = (reassigned || export_name)
? b`${`$$subscribe_${name}`}()`
: b`@component_subscribe($$self, ${name}, #value => $$invalidate('${value}', ${value} = #value))`;
Expand Down Expand Up @@ -425,12 +425,9 @@ export default function dom(
`);
}

const prop_names = x`[]`;

// TODO find a more idiomatic way of doing this
props.forEach(v => {
(prop_names as any).elements.push({ type: 'Literal', value: v.export_name });
});
const prop_names = x`{
${props.map(v => p`${v.export_name}: ${v.export_name === v.name ? 0 : x`"${v.name}"`}}`)}
}` as ObjectExpression;

if (options.customElement) {
const declaration = b`
Expand Down
20 changes: 11 additions & 9 deletions src/runtime/internal/Component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface T$$ {
update: () => void;
callbacks: any;
after_update: any[];
props: any;
props: Record<string, 0 | string>;
fragment: null|any;
not_equal: any;
before_update: any[];
Expand All @@ -22,9 +22,11 @@ interface T$$ {
}

export function bind(component, name, callback) {
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 •
edited
Loading

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

@Rich-Harris Rich-Harris Oct 28, 2019

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 •
edited
Loading

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

@Rich-Harris Rich-Harris Oct 28, 2019

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

name = component.$$.props[name] || name;
component.$$.bound[name] = callback;
callback(component.$$.ctx[name]);
}
}

export function mount_component(component, target, anchor) {
Expand Down Expand Up @@ -70,18 +72,18 @@ function make_dirty(component, key) {
component.$$.dirty[key] = true;
}

export function init(component, options, instance, create_fragment, not_equal, prop_names) {
export function init(component, options, instance, create_fragment, not_equal, props) {
const parent_component = current_component;
set_current_component(component);

const props = options.props || {};
const prop_values = options.props || {};

const $$: T$$ = component.$$ = {
fragment: null,
ctx: null,

// state
props: prop_names,
props,
update: noop,
not_equal,
bound: blank_object(),
Expand All @@ -101,14 +103,14 @@ export function init(component, options, instance, create_fragment, not_equal, p
let ready = false;

$$.ctx = instance
? instance(component, props, (key, ret, value = ret) => {
? instance(component, prop_values, (key, ret, value = ret) => {
if ($$.ctx && not_equal($$.ctx[key], $$.ctx[key] = value)) {
if ($$.bound[key]) $$.bound[key](value);
if (ready) make_dirty(component, key);
}
return ret;
})
: props;
: prop_values;

$$.update();
ready = true;
Expand Down
4 changes: 2 additions & 2 deletions test/js/samples/action-custom-event-handler/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function handleFoo(bar) {
}

function foo(node, callback) {

}

function instance($$self, $$props, $$invalidate) {
Expand All @@ -56,7 +56,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["bar"]);
init(this, options, instance, create_fragment, safe_not_equal, { bar: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/action/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function link(node) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, null, create_fragment, safe_not_equal, []);
init(this, options, null, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/bind-online/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/bind-open/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["open"]);
init(this, options, instance, create_fragment, safe_not_equal, { open: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/bind-width-height/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["w", "h"]);
init(this, options, instance, create_fragment, safe_not_equal, { w: 0, h: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/capture-inject-dev-only/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/collapses-text-around-comments/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class Component extends SvelteComponent {
constructor(options) {
super();
if (!document.getElementById("svelte-1a7i8ec-style")) add_css();
init(this, options, instance, create_fragment, safe_not_equal, ["foo"]);
init(this, options, instance, create_fragment, safe_not_equal, { foo: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-static-array/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function instance($$self) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-static-immutable/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function instance($$self) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, not_equal, []);
init(this, options, instance, create_fragment, not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-static-immutable2/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function instance($$self) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, not_equal, []);
init(this, options, instance, create_fragment, not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-static-var/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-static/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function instance($$self) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-store-access-invalidate/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-store-file-invalidate/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["increment"]);
init(this, options, instance, create_fragment, safe_not_equal, { increment: 0 });
}

get increment() {
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/component-store-reassign-invalidate/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/computed-collapsed-if/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["x", "a", "b"]);
init(this, options, instance, create_fragment, safe_not_equal, { x: 0, a: 0, b: 0 });
}

get a() {
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/css-media-query/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Component extends SvelteComponent {
constructor(options) {
super();
if (!document.getElementById("svelte-1slhpfn-style")) add_css();
init(this, options, null, create_fragment, safe_not_equal, []);
init(this, options, null, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/css-shadow-dom-keyframes/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Component extends SvelteElement {
constructor(options) {
super();
this.shadowRoot.innerHTML = `<style>div{animation:foo 1s}@keyframes foo{0%{opacity:0}100%{opacity:1}}</style>`;
init(this, { target: this.shadowRoot }, null, create_fragment, safe_not_equal, []);
init(this, { target: this.shadowRoot }, null, create_fragment, safe_not_equal, {});

if (options) {
if (options.target) {
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/data-attribute/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["bar"]);
init(this, options, instance, create_fragment, safe_not_equal, { bar: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/debug-empty/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponentDev {
constructor(options) {
super(options);
init(this, options, instance, create_fragment, safe_not_equal, ["name"]);
init(this, options, instance, create_fragment, safe_not_equal, { name: 0 });

dispatch_dev("SvelteRegisterComponent", {
component: this,
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/debug-foo-bar-baz-things/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponentDev {
constructor(options) {
super(options);
init(this, options, instance, create_fragment, safe_not_equal, ["things", "foo", "bar", "baz"]);
init(this, options, instance, create_fragment, safe_not_equal, { things: 0, foo: 0, bar: 0, baz: 0 });

dispatch_dev("SvelteRegisterComponent", {
component: this,
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/debug-foo/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponentDev {
constructor(options) {
super(options);
init(this, options, instance, create_fragment, safe_not_equal, ["things", "foo"]);
init(this, options, instance, create_fragment, safe_not_equal, { things: 0, foo: 0 });

dispatch_dev("SvelteRegisterComponent", {
component: this,
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/debug-hoisted/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function instance($$self) {
class Component extends SvelteComponentDev {
constructor(options) {
super(options);
init(this, options, instance, create_fragment, safe_not_equal, []);
init(this, options, instance, create_fragment, safe_not_equal, {});

dispatch_dev("SvelteRegisterComponent", {
component: this,
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/debug-no-dependencies/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function create_fragment(ctx) {
class Component extends SvelteComponentDev {
constructor(options) {
super(options);
init(this, options, null, create_fragment, safe_not_equal, []);
init(this, options, null, create_fragment, safe_not_equal, {});

dispatch_dev("SvelteRegisterComponent", {
component: this,
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/deconflict-builtins/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["createElement"]);
init(this, options, instance, create_fragment, safe_not_equal, { createElement: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/deconflict-globals/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["foo"]);
init(this, options, instance, create_fragment, safe_not_equal, { foo: 0 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/dev-warning-missing-data-computed/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponentDev {
constructor(options) {
super(options);
init(this, options, instance, create_fragment, safe_not_equal, ["foo"]);
init(this, options, instance, create_fragment, safe_not_equal, { foo: 0 });

dispatch_dev("SvelteRegisterComponent", {
component: this,
Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/dont-invalidate-this/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function make_uppercase() {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, null, create_fragment, safe_not_equal, []);
init(this, options, null, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/dynamic-import/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const func = () => import("./Foo.svelte");
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, null, create_fragment, safe_not_equal, []);
init(this, options, null, create_fragment, safe_not_equal, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/js/samples/each-block-array-literal/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["a", "b", "c", "d", "e"]);
init(this, options, instance, create_fragment, safe_not_equal, { a: 0, b: 0, c: 0, d: 0, e: 0 });
}
}

Expand Down
8 changes: 7 additions & 1 deletion test/js/samples/each-block-changed-check/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,13 @@ function instance($$self, $$props, $$invalidate) {
class Component extends SvelteComponent {
constructor(options) {
super();
init(this, options, instance, create_fragment, safe_not_equal, ["comments", "elapsed", "time", "foo"]);

init(this, options, instance, create_fragment, safe_not_equal, {
comments: 0,
elapsed: 0,
time: 0,
foo: 0
});
}
}

Expand Down
Loading