Merge pull request #4078 from sveltejs/bitmask-overflow-slot

Handle slot updates where parent component has many variables
5 years ago · 1f8472262f
parent 691211aeba 82ac963c07
commit 1f8472262f
12 changed files with 362 additions and 58 deletions
--- a/.eslintignore
+++ b/.eslintignore
@ -1,5 +1,6 @@
 **/_actual.js
 **/expected.js
+**/_output/**.js
 test/*/samples/*/output.js
 node_modules

--- a/.gitignore
+++ b/.gitignore
@ -23,6 +23,7 @@ node_modules
 /test/sourcemaps/samples/*/output.css.map
 /yarn-error.log
 _actual*.*
+_output
 /types

 /site/cypress/screenshots/
--- a/package-lock.json
+++ b/package-lock.json
@ -500,9 +500,9 @@
      "dev": true
    },
    "code-red": {
-      "version": "0.0.26",
-      "resolved": "https://registry.npmjs.org/code-red/-/code-red-0.0.26.tgz",
-      "integrity": "sha512-W4t68vk3xJjmkbuAKfEtaj7E+K82BtV+A4VjBlxHA6gDoSLc+sTB643JdJMSk27vpp5iEqHFuGnHieQGy/GmUQ==",
+      "version": "0.0.27",
+      "resolved": "https://registry.npmjs.org/code-red/-/code-red-0.0.27.tgz",
+      "integrity": "sha512-MSILIi8kkSGag76TW+PRfBP/dN++Ei+uTeiUfqW4Es5teFNrbsAWtyAbPwxKI1vxEsBx64loAfGxS1kVCo1d2g==",
      "dev": true,
      "requires": {
        "acorn": "^7.1.0",
--- a/package.json
+++ b/package.json
@ -64,7 +64,7 @@
    "acorn": "^7.1.0",
    "agadoo": "^1.1.0",
    "c8": "^5.0.1",
-    "code-red": "0.0.26",
+    "code-red": "0.0.27",
    "codecov": "^3.5.0",
    "css-tree": "1.0.0-alpha22",
    "eslint": "^6.3.0",
--- a/src/compiler/compile/render_dom/Renderer.ts
+++ b/src/compiler/compile/render_dom/Renderer.ts
@ -204,7 +204,6 @@ export default class Renderer {
 			? x`$$self.$$.dirty`
 			: x`#dirty`) as Identifier | MemberExpression;

-		let bitmask;
 		const get_bitmask = () => {
 			const bitmask: BitMasks = [];
 			names.forEach((name) => {
@ -228,48 +227,30 @@ export default class Renderer {
 			return bitmask;
 		};

-		let operator;
-		let left;
-		let right;
-
 		return {
-			get type() {
-				// we make the type a getter, even though it's always
-				// a BinaryExpression, because it gives us an opportunity
-				// to lazily create the node. TODO would be better if
-				// context was determined before rendering, so that
-				// this indirection was unnecessary
-				if (!bitmask) {
-					bitmask = get_bitmask();
-
-					if (!bitmask.length) {
-						({ operator, left, right } = x`${dirty} & /*${names.join(', ')}*/ 0` as BinaryExpression);
-					} else if (renderer.context_overflow) {
-						const expression = bitmask
-							.map((b, i) => ({ b, i }))
-							.filter(({ b }) => b)
-							.map(({ b, i }) => x`${dirty}[${i}] & /*${b.names.join(', ')}*/ ${b.n}`)
-							.reduce((lhs, rhs) => x`${lhs} | ${rhs}`);
-
-						({ operator, left, right } = expression as BinaryExpression);
-					} else {
-						({ operator, left, right } = x`${dirty} & /*${names.join(', ')}*/ ${bitmask[0].n}` as BinaryExpression);
-					}
+			// Using a ParenthesizedExpression allows us to create
+			// the expression lazily. TODO would be better if
+			// context was determined before rendering, so that
+			// this indirection was unnecessary
+			type: 'ParenthesizedExpression',
+			get expression() {
+				const bitmask = get_bitmask();
+
+				if (!bitmask.length) {
+					return x`${dirty} & /*${names.join(', ')}*/ 0` as BinaryExpression;
 				}

+				if (renderer.context_overflow) {
+					return bitmask
+						.map((b, i) => ({ b, i }))
+						.filter(({ b }) => b)
+						.map(({ b, i }) => x`${dirty}[${i}] & /*${b.names.join(', ')}*/ ${b.n}`)
+						.reduce((lhs, rhs) => x`${lhs} | ${rhs}`);
+				}

-				return 'BinaryExpression';
-			},
-			get operator() {
-				return operator;
-			},
-			get left() {
-				return left;
-			},
-			get right() {
-				return right;
+				return x`${dirty} & /*${names.join(', ')}*/ ${bitmask[0].n}` as BinaryExpression;
 			}
-		} as Expression;
+		} as any;
 	}

 	reference(node: string | Identifier | MemberExpression) {
--- a/src/compiler/compile/render_dom/wrappers/shared/get_slot_definition.ts
+++ b/src/compiler/compile/render_dom/wrappers/shared/get_slot_definition.ts
@ -2,6 +2,7 @@ import Let from '../../../nodes/Let';
 import { x, p } from 'code-red';
 import Block from '../../Block';
 import TemplateScope from '../../../nodes/shared/TemplateScope';
+import { BinaryExpression } from 'estree';

 export function get_slot_definition(block: Block, scope: TemplateScope, lets: Let[]) {
 	if (lets.length === 0) return { block, scope };
@ -28,21 +29,48 @@ export function get_slot_definition(block: Block, scope: TemplateScope, lets: Le
 		properties: Array.from(names).map(name => p`${block.renderer.context_lookup.get(name).index}: ${name}`)
 	};

-	const changes = Array.from(names)
-		.map(name => {
-			const { context_lookup } = block.renderer;
+	const { context_lookup } = block.renderer;

-			const literal = {
-				type: 'Literal',
-				get value() {
+	// i am well aware that this code is gross
+	// TODO make it less gross
+	const changes = {
+		type: 'ParenthesizedExpression',
+		get expression() {
+			if (block.renderer.context_overflow) {
+				const grouped = [];
+
+				Array.from(names).forEach(name => {
 					const i = context_lookup.get(name).index.value as number;
-					return 1 << i;
+					const g = Math.floor(i / 31);
+
+					if (!grouped[g]) grouped[g] = [];
+					grouped[g].push({ name, n: i % 31 });
+				});
+
+				const elements = [];
+
+				for (let g = 0; g < grouped.length; g += 1) {
+					elements[g] = grouped[g]
+						? grouped[g]
+							.map(({ name, n }) => x`${name} ? ${1 << n} : 0`)
+							.reduce((lhs, rhs) => x`${lhs} | ${rhs}`)
+						: x`0`;
 				}
-			};

-			return x`${name} ? ${literal} : 0`;
-		})
-		.reduce((lhs, rhs) => x`${lhs} | ${rhs}`);
+				return {
+					type: 'ArrayExpression',
+					elements
+				};
+			}
+
+			return Array.from(names)
+				.map(name => {
+					const i = context_lookup.get(name).index.value as number;
+					return x`${name} ? ${1 << i} : 0`;
+				})
+				.reduce((lhs, rhs) => x`${lhs} | ${rhs}`) as BinaryExpression;
+		}
+	};

 	return {
 		block,
--- a/src/runtime/internal/utils.ts
+++ b/src/runtime/internal/utils.ts
@ -77,9 +77,23 @@ export function get_slot_context(definition, ctx, $$scope, fn) {
 }

 export function get_slot_changes(definition, $$scope, dirty, fn) {
-	return definition[2] && fn
-		? $$scope.dirty | definition[2](fn(dirty))
-		: $$scope.dirty;
+	if (definition[2] && fn) {
+		const lets = definition[2](fn(dirty));
+
+		if (typeof $$scope.dirty === 'object') {
+			const merged = [];
+			const len = Math.max($$scope.dirty.length, lets.length);
+			for (let i = 0; i < len; i += 1) {
+				merged[i] = $$scope.dirty[i] | lets[i];
+			}
+
+			return merged;
+		}
+
+		return $$scope.dirty | lets;
+	}
+
+	return $$scope.dirty;
 }

 export function exclude_internal_props(props) {
--- a/test/helpers.js
+++ b/test/helpers.js
@ -1,6 +1,7 @@
 import * as jsdom from 'jsdom';
 import * as assert from 'assert';
 import * as glob from 'tiny-glob/sync.js';
+import * as path from 'path';
 import * as fs from 'fs';
 import * as colors from 'kleur';

@ -237,3 +238,16 @@ export function useFakeTimers() {
 		}
 	};
 }
+
+export function mkdirp(dir) {
+	const parent = path.dirname(dir);
+	if (parent === dir) return;
+
+	mkdirp(parent);
+
+	try {
+		fs.mkdirSync(dir);
+	} catch (err) {
+		// do nothing
+	}
+}
--- a/test/runtime/index.js
+++ b/test/runtime/index.js
@ -3,6 +3,7 @@ import * as path from "path";
 import * as fs from "fs";
 import { rollup } from 'rollup';
 import * as virtual from 'rollup-plugin-virtual';
+import * as glob from 'tiny-glob/sync.js';
 import { clear_loops, flush, set_now, set_raf } from "../../internal";

 import {
@ -10,7 +11,8 @@ import {
 	loadConfig,
 	loadSvelte,
 	env,
-	setupHtmlEqual
+	setupHtmlEqual,
+	mkdirp
 } from "../helpers.js";

 let svelte$;
@ -90,6 +92,33 @@ describe("runtime", () => {

 			const window = env();

+			glob('**/*.svelte', { cwd }).forEach(file => {
+				if (file[0] === '_') return;
+
+				const dir  = `${cwd}/_output/${hydrate ? 'hydratable' : 'normal'}`;
+				const out = `${dir}/${file.replace(/\.svelte$/, '.js')}`;
+
+				if (fs.existsSync(out)) {
+					fs.unlinkSync(out);
+				}
+
+				mkdirp(dir);
+
+				try {
+					const { js } = compile(
+						fs.readFileSync(`${cwd}/${file}`, 'utf-8'),
+						{
+							...compileOptions,
+							filename: file
+						}
+					);
+
+					fs.writeFileSync(out, js.code);
+				} catch (err) {
+					// do nothing
+				}
+			});
+
 			return Promise.resolve()
 				.then(() => {
 					// hack to support transition tests
@ -195,7 +224,7 @@ describe("runtime", () => {
 					} else {
 						throw err;
 					}
-				 }).catch(err => {
+				}).catch(err => {
 					failed.add(dir);
 					showOutput(cwd, compileOptions, compile); // eslint-disable-line no-console
 					throw err;
--- a/test/runtime/samples/bitmask-overflow-slot/Echo.svelte
+++ b/test/runtime/samples/bitmask-overflow-slot/Echo.svelte
@ -0,0 +1,5 @@
+<script>
+	export let dummy;
+</script>
+
+<slot dummy={dummy}></slot>
--- a/test/runtime/samples/bitmask-overflow-slot/_config.js
+++ b/test/runtime/samples/bitmask-overflow-slot/_config.js
@ -0,0 +1,124 @@
+export default {
+	html: `
+		<p>0</p>
+		<p>1</p>
+		<p>2</p>
+		<p>3</p>
+		<p>4</p>
+		<p>5</p>
+		<p>6</p>
+		<p>7</p>
+		<p>8</p>
+		<p>9</p>
+		<p>10</p>
+		<p>11</p>
+		<p>12</p>
+		<p>13</p>
+		<p>14</p>
+		<p>15</p>
+		<p>16</p>
+		<p>17</p>
+		<p>18</p>
+		<p>19</p>
+		<p>20</p>
+		<p>21</p>
+		<p>22</p>
+		<p>23</p>
+		<p>24</p>
+		<p>25</p>
+		<p>26</p>
+		<p>27</p>
+		<p>28</p>
+		<p>29</p>
+		<p>30</p>
+		<p>31</p>
+		<p>32</p>
+		<p>33</p>
+		<p>34</p>
+		<p>35</p>
+		<p>36</p>
+		<p>37</p>
+		<p>38</p>
+		<p>39</p>
+		<p>40</p>
+		<p>5:36</p>
+		<p>6:37</p>
+		<p>38</p>
+		<p>0</p>
+	`,
+
+	test({ assert, component, target }) {
+		component.reads = {};
+
+		component._0 = 'a';
+		component._30 = 'b';
+		component._31 = 'c';
+		component._32 = 'd';
+		component._40 = 'e';
+
+		component._5 = 'f';
+		component._6 = 'g';
+		component._36 = 'h';
+		component._37 = 'i';
+
+		assert.htmlEqual(target.innerHTML, `
+			<p>a</p>
+			<p>1</p>
+			<p>2</p>
+			<p>3</p>
+			<p>4</p>
+			<p>f</p>
+			<p>g</p>
+			<p>7</p>
+			<p>8</p>
+			<p>9</p>
+			<p>10</p>
+			<p>11</p>
+			<p>12</p>
+			<p>13</p>
+			<p>14</p>
+			<p>15</p>
+			<p>16</p>
+			<p>17</p>
+			<p>18</p>
+			<p>19</p>
+			<p>20</p>
+			<p>21</p>
+			<p>22</p>
+			<p>23</p>
+			<p>24</p>
+			<p>25</p>
+			<p>26</p>
+			<p>27</p>
+			<p>28</p>
+			<p>29</p>
+			<p>b</p>
+			<p>c</p>
+			<p>d</p>
+			<p>33</p>
+			<p>34</p>
+			<p>35</p>
+			<p>h</p>
+			<p>i</p>
+			<p>38</p>
+			<p>39</p>
+			<p>e</p>
+			<p>f:h</p>
+			<p>g:i</p>
+			<p>38</p>
+			<p>a</p>
+		`);
+
+		assert.deepEqual(component.reads, {
+			_0: 1,
+			_5: 3,
+			_6: 3,
+			_30: 1,
+			_31: 1,
+			_32: 1,
+			_36: 3,
+			_37: 3,
+			_40: 1
+		});
+	}
+};
--- a/test/runtime/samples/bitmask-overflow-slot/main.svelte
+++ b/test/runtime/samples/bitmask-overflow-slot/main.svelte
@ -0,0 +1,107 @@
+<script>
+	import Echo from './Echo.svelte';
+
+	export let reads = {};
+
+	export let _0 = '0';
+	export let _1 = '1';
+	export let _2 = '2';
+	export let _3 = '3';
+	export let _4 = '4';
+	export let _5 = '5';
+	export let _6 = '6';
+	export let _7 = '7';
+	export let _8 = '8';
+	export let _9 = '9';
+	export let _10 = '10';
+	export let _11 = '11';
+	export let _12 = '12';
+	export let _13 = '13';
+	export let _14 = '14';
+	export let _15 = '15';
+	export let _16 = '16';
+	export let _17 = '17';
+	export let _18 = '18';
+	export let _19 = '19';
+	export let _20 = '20';
+	export let _21 = '21';
+	export let _22 = '22';
+	export let _23 = '23';
+	export let _24 = '24';
+	export let _25 = '25';
+	export let _26 = '26';
+	export let _27 = '27';
+	export let _28 = '28';
+	export let _29 = '29';
+	export let _30 = '30';
+	export let _31 = '31';
+	export let _32 = '32';
+	export let _33 = '33';
+	export let _34 = '34';
+	export let _35 = '35';
+	export let _36 = '36';
+	export let _37 = '37';
+	export let _38 = '38';
+	export let _39 = '39';
+	export let _40 = '40';
+
+	$: foo = read(_6, '_6') + ':' + read(_37, '_37');
+	$: bar = read(_38, '_38');
+
+	const read = (value, label) => {
+		if (!reads[label]) reads[label] = 0;
+		reads[label] += 1;
+
+		return value;
+	};
+</script>
+
+<Echo dummy={_0} let:dummy>
+	<p>{read(_0, '_0')}</p>
+	<p>{read(_1, '_1')}</p>
+	<p>{read(_2, '_2')}</p>
+	<p>{read(_3, '_3')}</p>
+	<p>{read(_4, '_4')}</p>
+	<p>{read(_5, '_5')}</p>
+	<p>{read(_6, '_6')}</p>
+	<p>{read(_7, '_7')}</p>
+	<p>{read(_8, '_8')}</p>
+	<p>{read(_9, '_9')}</p>
+	<p>{read(_10, '_10')}</p>
+	<p>{read(_11, '_11')}</p>
+	<p>{read(_12, '_12')}</p>
+	<p>{read(_13, '_13')}</p>
+	<p>{read(_14, '_14')}</p>
+	<p>{read(_15, '_15')}</p>
+	<p>{read(_16, '_16')}</p>
+	<p>{read(_17, '_17')}</p>
+	<p>{read(_18, '_18')}</p>
+	<p>{read(_19, '_19')}</p>
+	<p>{read(_20, '_20')}</p>
+	<p>{read(_21, '_21')}</p>
+	<p>{read(_22, '_22')}</p>
+	<p>{read(_23, '_23')}</p>
+	<p>{read(_24, '_24')}</p>
+	<p>{read(_25, '_25')}</p>
+	<p>{read(_26, '_26')}</p>
+	<p>{read(_27, '_27')}</p>
+	<p>{read(_28, '_28')}</p>
+	<p>{read(_29, '_29')}</p>
+	<p>{read(_30, '_30')}</p>
+	<p>{read(_31, '_31')}</p>
+	<p>{read(_32, '_32')}</p>
+	<p>{read(_33, '_33')}</p>
+	<p>{read(_34, '_34')}</p>
+	<p>{read(_35, '_35')}</p>
+	<p>{read(_36, '_36')}</p>
+	<p>{read(_37, '_37')}</p>
+	<p>{read(_38, '_38')}</p>
+	<p>{read(_39, '_39')}</p>
+	<p>{read(_40, '_40')}</p>
+
+	<p>{read(_5, '_5') + ':' + read(_36, '_36')}</p>
+	<p>{foo}</p>
+	<p>{bar}</p>
+
+	<p>{dummy}</p>
+</Echo>