Skip to content

Commit

Permalink
deconflict conextual action variable (#5839)
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau authored Jan 4, 2021
1 parent 5949c4a commit 7342570
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 19 deletions.
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');
}

0 comments on commit 7342570

Please sign in to comment.