Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fit bitmask overflow initial dirty value #4507

Merged
merged 2 commits into from
Mar 5, 2020

Conversation

tanhauhau
Copy link
Member

@tanhauhau tanhauhau commented Mar 5, 2020

Fixes #4263

the initial dirty value needs to be depend on whether bitmask context overflow

@tanhauhau tanhauhau force-pushed the tanhauhau/bitmask-overflow-if branch from c7daef7 to 41a23a1 Compare March 5, 2020 01:06
@Conduitry Conduitry merged commit a829122 into sveltejs:master Mar 5, 2020
@Conduitry
Copy link
Member

Well GitHub apparently also attributed this one to me 🙁

@Conduitry
Copy link
Member

I've reopened the issue because, disconcertingly, the tests pass if you run them with npm test but bitmask-overflow-if fails with PUBLISH=true npm test which tests against the actual bundle that would be published. 😱 I have no idea.

@Conduitry
Copy link
Member

The compiler has different output in this test depending on whether it was build with PUBLISH=true. Without it, it generates current_block_type_index$ = select_block_type$(ctx, [-1]); - and in publish mode, it generates current_block_type_index$ = select_block_type$(ctx, -1);.

In the production/PUBLISH build, the method is compiled from

get_initial_dirty_bit() {
	const _this = this;
	// TODO: context-overflow make it less gross

	const val = x`-1`;
	return {
		...val,
		elements: [val],
		get type() {
			return _this.renderer.context_overflow ? 'ArrayExpression' : 'UnaryExpression';
		},
	};
}

to

get_initial_dirty_bit() {
	const _this = this;
	// TODO: context-overflow make it less gross
	const val = x `-1`;
	return Object.assign({}, val, { elements: [val], get type() {
					return _this.renderer.context_overflow ? 'ArrayExpression' : 'UnaryExpression';
			} });
}

i.e., the spread is compiled away.

According to some tests I just did, apparently Object.assign() will aggressively evaluate the getter, while the spread lets it remain lazy. Holy shit.

@tanhauhau tanhauhau deleted the tanhauhau/bitmask-overflow-if branch March 5, 2020 02:13
@tanhauhau
Copy link
Member Author

oh my.. we should really look into fixing those lazy code...
anyway, i have a patch for it temporarily #4508

hontas added a commit to hontas/svelte that referenced this pull request Mar 6, 2020
* 'master' of https://github.com/sveltejs/svelte: (137 commits)
  -> v3.19.2
  fix lazy code breaks in build
  fit bitmask overflow initial dirty value in 'if' blocks (sveltejs#4507)
  add dev runtime warning for unknown slot names (sveltejs#4501)
  fix render fallback slot content due to whitespace (sveltejs#4500)
  docs: describe falsy and nullish attribute behavior (sveltejs#4498)
  in spread, distinguish never-updating and always-updating props (sveltejs#4487)
  chore: more specific typings, and add README note about Yarn (sveltejs#4483)
  update changelog
  check for unknown props even if component doesn't have writable props (sveltejs#4454)
  Bump codecov from 3.5.0 to 3.6.5 (sveltejs#4433)
  fix bitmask overflow for slot (sveltejs#4485)
  mark module variables as mutated or reassigned (sveltejs#4469)
  docs: referenced_from_script var value (sveltejs#4486)
  docs: clarify default prop behaviour (sveltejs#4460)
  site: turn fancybutton into custombutton (sveltejs#4476)
  update changelog
  exclude global variables from $capture_state (sveltejs#4475)
  -> v3.19.1
  don't treat $$-names as stores during invalidation (sveltejs#4453)
  ...
taylorzane pushed a commit to taylorzane/svelte that referenced this pull request Dec 17, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong {#if} {:else} with context_overflow (this.context.length > 31)
2 participants