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

Revise RFC #389 with details about its argument #620

Merged
merged 3 commits into from
May 1, 2020

Conversation

chancancode
Copy link
Member

This PR revises RFC #389 with some additional details about its argument handling based on implementation experience in https://github.com/tildeio/ember-element-helper

@chancancode
Copy link
Member Author

cc @cibernox

@chancancode chancancode requested a review from wycats April 29, 2020 21:18

Example:

```hbs
{{#let (element @htmlTag) as |Tag|}}
<Tag>...</Tag>
<Tag class="my-element" {{on "click" this.tagClicked}}>Hello</Tag>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware this is probably not the most sensible (and accessible) example, but it's really just the simplest example to show that "modifiers get passed through" without too much bloat. Happy to change it to something else though.

chancancode added a commit to tildeio/ember-element-helper that referenced this pull request Apr 29, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request Apr 29, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request Apr 29, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request Apr 29, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request Apr 30, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request Apr 30, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
We propose to add a new `element` helper that takes a single positional argument.

* When passed a non-empty string it generates a contextual component that, when invoked, renders an element with the same tag name as the passed string, along with the passed attributes (if any), modifiers (if any) and yields to given block (if any).
* When passed an empty string, it generates a contextual compoment that, when invoked, yields to the given block without wrapping it an element and ignores any passed modifiers and attributes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite unspecified, this is how the proof-of-concept addon already behaves today. It's largely just a happy accident due to implementation uses an Ember.Component under-the-hood, but there are tests for this behavior and seems like a useful feature in general.


* When passed a non-empty string it generates a contextual component that, when invoked, renders an element with the same tag name as the passed string, along with the passed attributes (if any), modifiers (if any) and yields to given block (if any).
* When passed an empty string, it generates a contextual compoment that, when invoked, yields to the given block without wrapping it an element and ignores any passed modifiers and attributes.
* When passed `null` or `undefined`, it will return `null`.
Copy link
Member Author

@chancancode chancancode May 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proof-of-concept addon currently throws here. The motivation for this is to mostly match the (component) helper's behavior with this, and acknowledge that there are useful patterns arising from this. Note that when null is "invoked" it's a no-op, which is why this works for contextual components in general.

@rwjblue rwjblue merged commit bdac4a5 into master May 1, 2020
@rwjblue rwjblue deleted the dynamic-tag-names-revised branch May 1, 2020 19:32
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request May 6, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request May 6, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request May 6, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request May 6, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request May 6, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
chancancode added a commit to tildeio/ember-element-helper that referenced this pull request May 6, 2020
This commit updates the argument handling:

* To match [RFC PR #620](emberjs/rfcs#620)
  * Returns `null` on `null` or `undefined`
  * Use development mode assertions for other invalid arguments
  * Add test for modifiers usage
  * Add test for `...attributes` usage
* To match more closely the likely Ember side implementation
  * Handle argument errors at runtime
  * `owner.hasRegistration('helper:element') === true`

Since the argument errors are moved to runtime, this also removes
the need for separate node tests. Previously, that also caused an
error when `ember-auto-import` is included. Since we removed the
`qunit` dependency we can now include `ember-auto-import` to align
with the default addon blueprint.

See also https://discordapp.com/channels/480462759797063690/485447409296736276/704799818316251216

Closes #6
@chancancode
Copy link
Member Author

These changes are now available in ember-element-helper v0.3.1!

@nightire
Copy link

nightire commented May 6, 2020

What if I want to yield a dynamic element with pre-bound attributes?

{{yield (element (or @htmlTag "div") class="foo bar" (on "click" this.onClick))}}

Is the above snippet works?

Or maybe like this?

{{#let (element (or @htmlTag "div")) as |Tag|}}
  {{yield (component Tag class="foo bar" (on "click" this.onClick))}}
{{/let}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants