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

Fix incompatibility with {{each}} and frozen objects #26

Merged
merged 1 commit into from
Oct 17, 2017

Conversation

tomdale
Copy link
Contributor

@tomdale tomdale commented Oct 17, 2017

Prior to this commit, the Glimmer.js implementation of the VM's Iterable interface relied on using an UpdatableReference from @glimmer/object-reference. Unfortunately, @glimmer/object-reference was an experimental implementation that was never meant to be used outside an experimental context. Nonetheless, certain classes, particularly UpdatableReference have found their way in use throughout the system, including here and perhaps most noticeably throughout the Glimmer VM test suite. One of the shortcomings of the implementation is incompatibility with frozen objects, because it eagerly tries to store metadata on objects.

A production-ready version of the functionality in @glimmer/object-reference was written for tracked properties in @glimmer/component. Because we never made that functionality publicly available, however, people tended to just use the object-reference version.

This commit makes the following changes:

  1. Exposes RootReference, CachedReference, UpdatableReference and ConditionalReference from @glimmer/component.
  2. Modifies the @glimmer/application Iterable implementation to use this version of UpdatableReference.
  3. Adds a suite of integration tests for the {{each}} helper, testing frozen and non-frozen variations of string literals, number literals and objects.

In the future, we are planning to move the Reference subclasses provided by @glimmer/component into glimmer-vm so they are both more discoverable and more easily shared across host environments, particularly Ember.js and Glimmer.js.

Prior to this commit, the Glimmer.js implementation of the VM's `Iterable` interface relied on using an `UpdatableReference` from `@glimmer/object-reference`. Unfortunately, `@glimmer/object-reference` was an experimental implementation that was never meant to be used outside an experimental context. Nonetheless, certain classes, particularly `UpdatableReference` have found their way in use throughout the system, including here and perhaps most noticeably throughout the Glimmer VM test suite. One of the shortcomings of the implementation is incompatibility with frozen objects, because it eagerly tries to store metadata on objects.

A production-ready version of the functionality in `@glimmer/object-reference` was written for tracked properties in `@glimmer/component`. Because we never made that functionality publicly available, however, people tended to just use the `object-reference` version.

This commit makes the following changes:

1. Exposes `RootReference`, `CachedReference`, `UpdatableReference` and `ConditionalReference` from `@glimmer/component`.
2. Modifies the `@glimmer/application` `Iterable` implementation to use this version of `UpdatableReference`.
3. Adds a suite of integration tests for the `{{each}}` helper, testing frozen and non-frozen variations of string literals, number literals and objects.

In the future, we are planning to move the `Reference` subclasses provided by `@glimmer/component` into `glimmer-vm` so they are both more discoverable and more easily shared across host environments, particularly Ember.js and Glimmer.js.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants