[REFACTOR] Refactors while integrating with Ember #993

pzuraq · 2019-11-16T22:40:30Z

Changelog

Overall updates
- Added @glimmer/env to the build, usable from Ember as well.
- Converted to native symbol, with a simple polyfill
Updates to the Runtime builders (AotRuntime(),JitRuntime())
- Can now accept an options hash as the first argument containing
  either the document or append/update operations.
Updates to Environment
- Removed DefaultEnvironment
- Simplified environment inheritance tree, flattened down to one class
- Added willDestroy to environment for WILL_DROP (legacy sync drops)
Updates to the RuntimeEnvironmentDelegate:
- Rename to EnvironmentDelegate
- Added isInteractive, equivalent of Ember's usage (should update?)
- Added extra field, which is a generic type for the Environment. This
  allows the embedding environment to pass any extra values they need.
- Removed the iterables API, key for methods are now statically
  generated and entirely a concern of the IteratorImpl
- Added toIterator, which allows the delegate to provide an
  IteratorDelegate for a given value. To keep things simpler and
  more performant, this is not yet a true browser delegate. This way,
  it matches the API of the (now internal) IterationItem.
  - Note for future: key and memo names are fairly confusing,
    maybe memo -> index?
- Added getPath/setPath to allow Ember to provide a getter/setter
  implementation, allowing for PropertyReference to delegate property access,
  along with keyFor in iterators.
- Added setTemplatePathDebugContext/getTemplatePathDebugContext. These are
  used to add debug context for paths in templates (e.g. {{this.foo}} or
  {{my-helper}}) to the DebugRenderTree in Ember. Eventually they should
  be removed, when the DebugRenderTree has been pulled upstream into Glimmer.
Updates to IterableImpl
- Added the IteratorDelegate interface (could maybe use a better
  name). This is the minimal API to let Ember provide all the
  different types of iterators it needs to (in particular,
  EmberArray, objects with forEach, and {{each-in}} iterators).
- No more native iterables - we don't support Symbol yet, plus it's
  fairly redundant with IteratorDelegate.
- Very simple IteratorWrapper class instead, which adds the key
  to the final iteration item internally. No more need to expose
  keyFor concerns to Ember, except getPath.
Updates to @glimmer/runtime
- Added WILL_DROP to destroyables, and willDestroy to component managers
Updates to @glimmer/reference
- Removed MapReference
- Removed UpdatableReference, replaced with UpdatableRootReference and
  IterationItemReference
- Refactored the majority of references around two base assumptions.
  - Generally, references should refer to values in the template
  - There are fundamentally two kinds of references in the template: Roots
    and properties. Roots are unique, there are a few different types.
    Properties are almost all the same.
- All template references now receive the env, which allows them to delegate
  access, and setup debug context.
Updates to @glimmer/validator
- Added the autotracking transaction from Ember, along with an API for setting
  it up based on the embedding environment.
- Simplified the meta system, now it's just a few WeakMaps.
- Added DEBUG assertions against incorrect usage of tags, for instance
  updating a tag with a more recent tag in a way that wouldn't bust the cache.

- Updates to the Runtime builds (`AotRuntime()`,`JitRuntime()`) - Can now accept an options hash as the first argument containing either the document or append/update operations. - Updates to the RuntimeEnvironmentDelegate: - Removed the `iterables` API, key for methods are now statically generated and entirely a concern of the IteratorImpl - Added `toIterator`, which allows the delegate to provide an `IteratorDelegate` for a given value. To keep things simpler and more performant, this is not yet a true browser delegate. This way, it matches the API of the (now internal) `IterationItem`. - Note for future: `key` and `memo` names are fairly confusing, maybe `memo` -> `index`? - Added `getPath` to allow Ember to provide a getter implementation, allowing `keyFor` to be fully pluggable. This will probably also be necessary for `RootReference` if we want to use the upstream version provided by Glimmer now, since it needs to use `get` to autotrack correctly. - Updates to `IterableImpl` - Added the `IteratorDelegate` interface (could maybe use a better name). This is the minimal API to let Ember provide all the different types of iterators it needs to (in particular, `EmberArray`, objects with `forEach`, and `{{each-in}}` iterators). - No more native iterables - we don't support `Symbol` yet, plus it's fairly redundant with `IteratorDelegate`. - Very simple `IteratorWrapper` class instead, which adds the key to the final iteration item internally. No more need to expose `keyFor` concerns to Ember, except `getPath`.

…s, refactor delegate, add @glimmer/env to local build

* Add caching to PropertyReference * Add WILL_DROP, willDestroy capability * Fix opcode_compiler bug (staticComponent generation dropped positional) * Fix bug with iterable identity

…the context of the delegate

This allows local tests to use DEBUG and test against errors and assertions. This commit also fixes a few assertions, some of which could not work for one reason or another, and some of which needed tweaking. In particular, `@glimmer/validator` need a tweak in DEBUG mode. Our current assumption is that by the first time you call `value` on a tag in a render, that tag's value should truly be locked down, and you should not be able to change it without a call to `dirty()` somewhere. This allows us to cache the value. We also use a trick to allow us to compute the value _once_ during `validate()` in production builds - We know the global $REVISION cannot possibly be less than the value of the tag, so we return it directly. However, this does mean that it _is_ valid to update the value of a tag between a false `validate()` and `value()`, and this is expected, as we have a number of cases where we do this. This commit introduces an assertion that throws if you attempt to update a tag to a more recent value after it has been locked in, and in DEBUG it prevents caching while revalidating in order to allow that brief period between a false `validate()` and a `value()` to not trigger this assertion.

package.json

ember-cli-build.js

rwjblue · 2019-12-17T20:59:46Z

BREAKING.md

+
+# v0.45.0
+
+* `@glimmer/runtime`


Just a note to remind myself to copy this content into the changelog when we land / release this.

rwjblue · 2019-12-17T21:01:45Z

build/broccoli/build-tests.js

+  let glimmerEnv = funnel('node_modules/@glimmer/env', {
+    include: ['dist/amd/es5/*.{js,map}'],
+  });


Can you check if we can remove this?

rwjblue · 2019-12-17T21:52:46Z

packages/@glimmer/reference/lib/template.ts

+    super(env);
+
+    if (DEBUG) {
+      let name = debugName || fn.name;


This is a bit unfortunate, we should try to figure out how we can thread through the invoked helper name here.

Seems totally fine to followup with this though...

packages/@glimmer/reference/lib/template.ts

rwjblue · 2019-12-17T22:00:55Z

packages/@glimmer/reference/lib/template.ts

+  compute() {
+    this.computeTag = track(() => {
+      this.computeValue = this.fn(this.args);
+    }, DEBUG && `${this.env.getTemplatePathDebugContext(this)}`);


Suggested change

}, DEBUG && `${this.env.getTemplatePathDebugContext(this)}`);

}, DEBUG && this.env.getTemplatePathDebugContext(this));

rwjblue · 2019-12-17T22:38:15Z

packages/@glimmer/runtime/lib/environment.ts

-): JitRuntimeContext {
-  let env = new RuntimeEnvironment(document, new RuntimeEnvironmentDelegateImpl(delegate));
+  resolver: RuntimeResolverDelegate<R> = {},
+  delegate: EnvironmentDelegate<E> = {}


I think ideally we should try to make it so that the only thing in the system that is aware of the delegate setup that we are using is the environment. I don't particularly think that will make much difference here at the moment, but it does isolate unrelated parts of the system (like this one) from other future changes that we might make to the delegate.

tldr; lets pass the environment here instead of the delegate, but seems fine to do in a follow up...

packages/@glimmer/runtime/lib/lifetime.ts

rwjblue · 2019-12-17T22:47:04Z

packages/@glimmer/util/lib/lifetimes.ts

@@ -35,6 +38,10 @@ export function associateDestructor(parent: object, child: Drop): void {
  associated.add(child);
 }

+export function peekAssociated(parent: object): Option<Set<Drop>> {
+  return LINKED.get(parent) || null;


return LINKED_IN.get(parent) || null;

packages/@glimmer/validator/lib/utils.ts

pzuraq force-pushed the refactor/ember-integration branch from 9ce08fc to 3816858 Compare November 16, 2019 23:03

Chris Garrett added 9 commits November 25, 2019 10:24

more fixes for environment

0832b95

updates to Environment types

63eaab0

TEMPORARY fixes to get the build working DROP THIS LATER

84fce15

refactor references, update environment to include reference function…

49f5cb6

…s, refactor delegate, add @glimmer/env to local build

update tracking, add transaction

0c79d9c

update references

1e16595

updates to toIterator, tracking assertion, template path, cleanup

3be1a9d

update to symbol with polyfill

68ea5cb

* Fix autotracking error message

ccea119

* Add caching to PropertyReference * Add WILL_DROP, willDestroy capability * Fix opcode_compiler bug (staticComponent generation dropped positional) * Fix bug with iterable identity

pzuraq force-pushed the refactor/ember-integration branch from 55704ad to 2528ecc Compare December 13, 2019 18:11

fix types and lint

bc361e5

pzuraq force-pushed the refactor/ember-integration branch from 2528ecc to bc361e5 Compare December 13, 2019 18:30

Chris Garrett added 7 commits December 13, 2019 12:13

Cleanup Environment, add onTransactionBegin and onTransactionCommit

7412dd3

cleanup Environment TemplateContext methods -> should always call in …

24a2518

…the context of the delegate

add binding to null for delegate methods

8f82111

final round

1fa71a7

fix lifetimes, etc

fb3908a

start working on tests

5badc6e

rwjblue reviewed Dec 16, 2019

View reviewed changes

package.json Show resolved Hide resolved

rwjblue reviewed Dec 16, 2019

View reviewed changes

ember-cli-build.js Outdated Show resolved Hide resolved

Chris Garrett added 5 commits December 16, 2019 17:55

add tests

dd907d5

simplify build

957ca15

add back yarn to volta

45753a0

update BREAKING

10c4fba

fix types, remove WeakSet usage

7e98fdc

pzuraq changed the title ~~[WIP][REFACTOR] Refactors while integrating with Ember~~ [REFACTOR] Refactors while integrating with Ember Dec 17, 2019

pzuraq marked this pull request as ready for review December 17, 2019 03:51

Chris Garrett added 2 commits December 17, 2019 12:21

Object.values is not supported in IE11

c949de5

IE11 doesn't support function names

f179620

rwjblue added the breaking label Dec 17, 2019

rwjblue approved these changes Dec 17, 2019

View reviewed changes

address comments

d73dde0

pzuraq force-pushed the refactor/ember-integration branch from 7b40e91 to d73dde0 Compare December 17, 2019 23:32

rwjblue merged commit ee0b970 into master Dec 18, 2019

rwjblue deleted the refactor/ember-integration branch December 18, 2019 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] Refactors while integrating with Ember #993

[REFACTOR] Refactors while integrating with Ember #993

pzuraq commented Nov 16, 2019 •

edited

rwjblue Dec 17, 2019

rwjblue Dec 17, 2019

rwjblue Dec 17, 2019

rwjblue Dec 17, 2019

rwjblue Dec 17, 2019

rwjblue Dec 17, 2019

	}, DEBUG && `${this.env.getTemplatePathDebugContext(this)}`);
	}, DEBUG && this.env.getTemplatePathDebugContext(this));

[REFACTOR] Refactors while integrating with Ember #993

[REFACTOR] Refactors while integrating with Ember #993

Conversation

pzuraq commented Nov 16, 2019 • edited

Changelog

rwjblue Dec 17, 2019

Choose a reason for hiding this comment

rwjblue Dec 17, 2019

Choose a reason for hiding this comment

rwjblue Dec 17, 2019

Choose a reason for hiding this comment

rwjblue Dec 17, 2019

Choose a reason for hiding this comment

rwjblue Dec 17, 2019

Choose a reason for hiding this comment

rwjblue Dec 17, 2019

Choose a reason for hiding this comment

pzuraq commented Nov 16, 2019 •

edited