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

deconflict variable for action and each block #5839

Merged
merged 5 commits into from
Jan 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Rework SSR store handling to subscribe and unsubscribe as in DOM mode ([#3375](https://github.com/sveltejs/svelte/issues/3375), [#3582](https://github.com/sveltejs/svelte/issues/3582), [#3636](https://github.com/sveltejs/svelte/issues/3636))
* Fix error when removing elements that are already transitioning out ([#5789](https://github.com/sveltejs/svelte/issues/5789), [#5808](https://github.com/sveltejs/svelte/issues/5808))
* Fix duplicate content race condition with `{#await}` blocks and out transitions ([#5815](https://github.com/sveltejs/svelte/issues/5815))
* Deconflict variable names used for contextual actions ([#5839](https://github.com/sveltejs/svelte/issues/5839))

## 3.31.1

Expand Down
7 changes: 5 additions & 2 deletions src/compiler/compile/nodes/Action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ import Node from './shared/Node';
import Expression from './shared/Expression';
import Component from '../Component';
import TemplateScope from './shared/TemplateScope';
import { TemplateNode } from '../../interfaces';
import { Directive } from '../../interfaces';

export default class Action extends Node {
type: 'Action';
name: string;
expression: Expression;
uses_context: boolean;
template_scope: TemplateScope;

constructor(component: Component, parent: Node, scope: TemplateScope, info: TemplateNode) {
constructor(component: Component, parent: Node, scope: TemplateScope, info: Directive) {
super(component, parent, scope, info);

const object = info.name.split('.')[0];
Expand All @@ -23,6 +24,8 @@ export default class Action extends Node {
? new Expression(component, this, scope, info.expression)
: null;

this.template_scope = scope;

this.uses_context = this.expression && this.expression.uses_context;
}
}
16 changes: 1 addition & 15 deletions src/compiler/compile/nodes/shared/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Node, FunctionExpression, Identifier } from 'estree';
import { INode } from '../interfaces';
import { is_reserved_keyword } from '../../utils/reserved_keywords';
import replace_object from '../../utils/replace_object';
import is_contextual from './is_contextual';
import EachBlock from '../EachBlock';

type Owner = INode;
Expand Down Expand Up @@ -409,18 +410,3 @@ function get_function_name(_node, parent) {

return 'func';
}

function is_contextual(component: Component, scope: TemplateScope, name: string) {
if (is_reserved_keyword(name)) return true;

// if it's a name below root scope, it's contextual
if (!scope.is_top_level(name)) return true;

const variable = component.var_lookup.get(name);

// hoistables, module declarations, and imports are non-contextual
if (!variable || variable.hoistable) return false;

// assume contextual
return true;
}
18 changes: 18 additions & 0 deletions src/compiler/compile/nodes/shared/is_contextual.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import Component from '../../Component';
import TemplateScope from './TemplateScope';
import { is_reserved_keyword } from '../../utils/reserved_keywords';

export default function is_contextual(component: Component, scope: TemplateScope, name: string) {
if (is_reserved_keyword(name)) return true;

// if it's a name below root scope, it's contextual
if (!scope.is_top_level(name)) return true;

const variable = component.var_lookup.get(name);

// hoistables, module declarations, and imports are non-contextual
if (!variable || variable.hoistable) return false;

// assume contextual
return true;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { b, x } from 'code-red';
import Block from '../../Block';
import Action from '../../../nodes/Action';
import is_contextual from '../../../nodes/shared/is_contextual';

export default function add_actions(
block: Block,
Expand All @@ -11,7 +12,7 @@ export default function add_actions(
}

export function add_action(block: Block, target: string, action: Action) {
const { expression } = action;
const { expression, template_scope } = action;
let snippet;
let dependencies;

Expand All @@ -28,7 +29,9 @@ export function add_action(block: Block, target: string, action: Action) {

const [obj, ...properties] = action.name.split('.');

const fn = block.renderer.reference(obj);
const fn = is_contextual(action.component, template_scope, obj)
? block.renderer.reference(obj)
: obj;

if (properties.length) {
const member_expression = properties.reduce((lhs, rhs) => x`${lhs}.${rhs}`, fn);
Expand Down
13 changes: 13 additions & 0 deletions test/runtime/samples/deconflict-contextual-action/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
let result;

export default {
before_test() {
result = [];
},
props: {
collect: (str) => result.push(str)
},
test({ assert }) {
assert.deepEqual(result, ['each_action', 'import_action']);
}
};
17 changes: 17 additions & 0 deletions test/runtime/samples/deconflict-contextual-action/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
import action from './util.js';
export let collect;

function each_action(_, fn) {
fn('each_action');
}
const array = [each_action];
</script>

<div use:action={collect} />

<ul>
{#each array as action}
<div use:action={collect} />
{/each}
</ul>
3 changes: 3 additions & 0 deletions test/runtime/samples/deconflict-contextual-action/util.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function (_, fn) {
fn('import_action');
}