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

Ensure nested custom elements fallback properly. #488

Merged
merged 3 commits into from
May 17, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented May 15, 2017

This fixes an issue with Ember versions using [email protected] when attempting to use nested web components. This is obviously not correct for angle bracket components but since no consumers are currently using them with [email protected] we are hard coding support to just use the fallback case.

The master version of glimmer-vm (v0.23.0-alpha.x) properly handles this without issue (for either true angle bracket invocation or web component fallback invocation).

Thanks for @krisselden for pairing with me to teach me how to do this.

@rwjblue rwjblue changed the base branch from master to release-0-22 May 15, 2017 19:45
This fixes an issue with Ember versions using [email protected] when attempting
to use nested web components.  This is obviously not correct for angle bracket components
but since no consumers are currently using them with [email protected] we are hard coding
support to just use the fallback case.

The master version of glimmer-vm (v0.23.0-alpha.x) properly handles this without issue
(for either true angle bracket invocation or web component fallback invocation).

diff --git a/packages/@glimmer/runtime/lib/syntax/functions.ts b/packages/@glimmer/runtime/lib/syntax/functions.ts
index 3c9b3f2..29a2bba 100644
--- a/packages/@glimmer/runtime/lib/syntax/functions.ts
+++ b/packages/@glimmer/runtime/lib/syntax/functions.ts
@@ -196,6 +196,29 @@ STATEMENTS.add(Ops.ScannedBlock, (sexp: BaselineSyntax.ScannedBlock, builder) =>
   blocks.compile([Ops.NestedBlock, path, params, hash, templateBlock, inverseBlock], builder);
 });

+// this fixes an issue with Ember versions using [email protected] when attempting
+// to use nested web components.  This is obviously not correct for angle bracket components
+// but since no consumers are currently using them with [email protected] we are hard coding
+// support to just use the fallback case.
+STATEMENTS.add(Ops.Component, (sexp: WireFormat.Statements.Component, builder) => {
+  let [, tag, component ] = sexp;
+  let { attrs, statements } = component;
+
+  builder.openPrimitiveElement(tag);
+
+  for (let i = 0; i < attrs.length; i++) {
+    STATEMENTS.compile(attrs[i], builder);
+  }
+
+  builder.flushElement();
+
+  for (let i = 0; i < statements.length; i++) {
+    STATEMENTS.compile(statements[i], builder);
+  }
+
+  builder.closeElement();
+});
+
 STATEMENTS.add(Ops.ScannedComponent, (sexp: BaselineSyntax.ScannedComponent, builder) => {
   let [, tag, attrs, rawArgs, rawBlock] = sexp;
   let block = rawBlock && rawBlock.scan();
diff --git a/packages/@glimmer/runtime/tests/rendering/content-test.ts b/packages/@glimmer/runtime/tests/rendering/content-test.ts
index c06ad9e..d39aaad 100644
--- a/packages/@glimmer/runtime/tests/rendering/content-test.ts
+++ b/packages/@glimmer/runtime/tests/rendering/content-test.ts
@@ -29,6 +29,19 @@ class StaticContentTests extends RenderingTest {
     this.assertStableRerender();
   }

+  @template(`<use-the-platform><seriously-please data-foo="1">Stuff <div>Here</div></seriously-please></use-the-platform>`)
+  ['renders nested custom elements']() {
+    this.render({});
+    this.assertContent(`<use-the-platform><seriously-please data-foo="1">Stuff <div>Here</div></seriously-please></use-the-platform>`);
+    this.assertStableRerender();
+  }
+  @template(`<use-the-platform><seriously-please data-foo="1"><wheres-the-platform>Here</wheres-the-platform></seriously-please></use-the-platform>`)
+  ['renders MOAR NESTED custom elements']() {
+    this.render({});
+    this.assertContent(`<use-the-platform><seriously-please data-foo="1"><wheres-the-platform>Here</wheres-the-platform></seriously-please></use-the-platform>`);
+    this.assertStableRerender();
+  }
+
   @template(strip`
     <div class="world">
       <h1>Hello World!</h1>
@rwjblue
Copy link
Member Author

rwjblue commented May 15, 2017

CI is failing due to some lerna stuff I guess?

@rwjblue rwjblue merged commit 0f8d3ce into glimmerjs:release-0-22 May 17, 2017
@rwjblue rwjblue deleted the fix-custom-elements branch May 17, 2017 19:12
rwjblue added a commit to rwjblue/ember.js that referenced this pull request May 17, 2017
A bug was introduced with the Glimmer update that was included in
Ember 2.13 that caused an error to be thrown when using nested
custom elements like:

```hbs
<foo-bar>
  <baz-qux></baz-qux>
</foo-bar>
```

The fix was done upstream in glimmerjs/glimmer-vm#488.
@backspace
Copy link

Thank you! 💞

@chadhietala
Copy link
Member

I'm sorry you had to fight the Lerna.
image

rwjblue added a commit to emberjs/ember.js that referenced this pull request May 17, 2017
A bug was introduced with the Glimmer update that was included in
Ember 2.13 that caused an error to be thrown when using nested
custom elements like:

```hbs
<foo-bar>
  <baz-qux></baz-qux>
</foo-bar>
```

The fix was done upstream in glimmerjs/glimmer-vm#488.

(cherry picked from commit 15c2d25)
rwjblue added a commit to emberjs/ember.js that referenced this pull request May 17, 2017
A bug was introduced with the Glimmer update that was included in
Ember 2.13 that caused an error to be thrown when using nested
custom elements like:

```hbs
<foo-bar>
  <baz-qux></baz-qux>
</foo-bar>
```

The fix was done upstream in glimmerjs/glimmer-vm#488.

(cherry picked from commit 15c2d25)
@Turbo87 Turbo87 added the bug label Oct 22, 2017
@Turbo87 Turbo87 changed the title [BUGFIX release] Ensure nested custom elements fallback properly. Ensure nested custom elements fallback properly. Oct 22, 2017
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.

4 participants