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

Make StreamElement.templateElement more flexible #665

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

seanpdoyle
Copy link
Contributor

Prior to this commit, the StreamElement.templateElement property would
raise an error if it were called without a <template> descendant.

However, a <turbo-stream action="remove"> element without a descendant
<template> element is valid and supported.

When called on a <turbo-stream> that has a firstElementChild that
is not a <template> element will continue to raise an exception.

When called without any child elements,
StreamElement.templateElement will create and append a <template>
element.

Since listeners to the turbo:before-stream-render event now have
access to the StreamElement creating, appending, and returning a
<template> element will provide them with an opportunity to manipulate
the element from their event listener if they choose to.

Prior to this commit, the `StreamElement.templateElement` property would
raise an error if it were called without a `<template>` descendant.

However, a `<turbo-stream action="remove">` element without a descendant
`<template>` element is valid and supported.

When called on a `<turbo-stream>` that has a `firstElementChild` that
_is not_ a `<template>` element will continue to raise an exception.

When called without _any_ child elements,
`StreamElement.templateElement` will create and append a `<template>`
element.

Since listeners to the `turbo:before-stream-render` event now have
access to the `StreamElement` creating, appending, and returning a
`<template>` element will provide them with an opportunity to manipulate
the element from their event listener if they choose to.
@davekaro
Copy link

davekaro commented Aug 4, 2022

This fixes #664

Copy link

@marcelolx marcelolx left a comment

Choose a reason for hiding this comment

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

You rock @seanpdoyle!

@dhh dhh merged commit bff30b3 into hotwired:main Aug 4, 2022
@seanpdoyle seanpdoyle deleted the relax-stream-template-validation branch August 4, 2022 13:24
@excid3
Copy link
Contributor

excid3 commented Aug 10, 2022

Just ran into this as well. Any chance we could get a beta3 release with this?

@seanpdoyle
Copy link
Contributor Author

@excid3
Copy link
Contributor

excid3 commented Aug 10, 2022

Sure can. 👍

@marcelolx
Copy link

@excid3 if you're using turbo-rails see this message https://discord.com/channels/988103760050012160/988104775079968849/1006524274540023808

⬇️ (if you don't want to open discord)

For those that want to test the beta 2 but are blocked because of this problem and may be waiting for a release (and depend on turbo-rails), we have a branch with Turbo updated to the latest release that is available in the dev-builds repo

https://github.com/marcelolx/turbo-rails/tree/%40hotwired/turbo/c4e0aba

Change your package.json to (if node env)
"@hotwired/turbo-rails": "marcelolx/turbo-rails.git#@hotwired/turbo/c4e0aba"

And your Gemfile to
gem "turbo-rails", github: "marcelolx/turbo-rails", branch: "@hotwired/turbo/c4e0aba"

@excid3
Copy link
Contributor

excid3 commented Aug 10, 2022

Awesome, thanks @marcelolx 👍

@rbclark
Copy link

rbclark commented Sep 12, 2022

Just as a note, if anyone is using importmaps and wants to target a version that includes this fix, I've done so using
https://cdn.jsdelivr.net/gh/hotwired/dev-builds@35faf826645b4f57f62f8386904a0bc8d6969dc6/dist/turbo.es2017-esm.js

It turns out that targeting files directly on Github doesn't work however jdselivr.net provides a proxy.

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

Successfully merging this pull request may close these issues.

6 participants