Activate <script> for GET Form Submissions
#815
Comments
tbcooney
changed the title
<script> for GET Form Submissions<script> for GET Form Submissions
First of all, I would expect a I'm not sure of the exact comment you're referring to, but as far as considering code in this style as an anti-pattern goes, I think it could be improved by adopting Stimulus and some Stimulus idioms. For example, introducing a import { Controller } from "@hotwired/stimulus"
export default class extends Controller {
connect() {
this.createChart()
}
createChart() {
// ...
}
}Then, you can route the -<turbo-frame id="sales" src="http://lvh.me:5000/reports/sales?start_date=2022-11-01&end_date=2022-11-04" complete="">
+<turbo-frame id="sales" src="http://lvh.me:5000/reports/sales?start_date=2022-11-01&end_date=2022-11-04" complete=""
+ data-controller="chartkick" data-action="chartkick:load@window->chartkick#createChart">
<div id="chart-1" style="height: 280px; width: 100%; text-align: center; color: #999; line-height: 280px; font-size: 14px; font-family: 'Lucida Grande', 'Lucida Sans Unicode', Verdana, Arial, Helvetica, sans-serif;">
<canvas style="box-sizing: border-box;" width="300"></canvas>
</div>
</turbo-frame>With that controller in place, you could render the chart's data as JSON inside a Stimulus Value or as the text content of a import { Controller } from "@hotwired/stimulus"
export default class extends Controller {
static targets = ["chart", "data", "configuration"]
connect() {
this.createChart()
}
createChart() {
if (document.documentElement.hasAttribute("data-turbolinks-preview")) return
if (document.documentElement.hasAttribute("data-turbo-preview")) return
if ("Chartkick" in window && this.hasChartTarget && this.hasDataTarget && this.hasConfigurationTarget) {
const chart = this.chartTarget.id
const data = JSON.parse(this.dataTarget.text)
const configuration = JSON.parse(this.configurationTarget.text)
new Chartkick["ColumnChart"](this.chartTarget, data, configuration)
}
}
}Then, you could render what's necessary into the HTML: <turbo-frame id="sales" src="http://lvh.me:5000/reports/sales?start_date=2022-11-01&end_date=2022-11-04" complete=""
data-controller="chartkick" data-action="chartkick:load@window->chartkick#createChart">
- <div id="chart-1" style="height: 280px; width: 100%; text-align: center; color: #999; line-height: 280px; font-size: 14px; font-family: 'Lucida Grande', 'Lucida Sans Unicode', Verdana, Arial, Helvetica, sans-serif;">
+ <div id="chart-1" style="height: 280px; width: 100%; text-align: center; color: #999; line-height: 280px; font-size: 14px; font-family: 'Lucida Grande', 'Lucida Sans Unicode', Verdana, Arial, Helvetica, sans-serif;"
+ data-chartkick-target="chart">
<canvas style="box-sizing: border-box;" width="300"></canvas>
</div>
+ <script type="application/json" data-chartkick-target="data"><%= ... %></script>
+ <script type="application/json" data-chartkick-target="configuration"><%= ... %></script>
</turbo-frame>Since the Then, since your JavaScript logic is out of your templates and into your Asset Pipeline (or other build chain), you have all the power of JavaScript classes at your disposal, including extracting properties: createChart() {
if (this.pageIsPreview) return
if (this.canRender) {
new Chartkick["ColumnChart"](...this.chartkickArguments)
}
}
get chartkickArguments() {
return [
this.chartTarget.id,
JSON.parse(this.dataTarget.text),
JSON.parse(this.configurationTarget.text)
]
}
get pageIsPreview() {
return document.documentElement.hasAttribute("data-turbolinks-preview") ||
document.documentElement.hasAttribute("data-turbo-preview")
}
get canRender() {
return "Chartkick" in window && this.hasChartTarget && this.hasDataTarget && this.hasConfigurationTarget
} |
|
@tbcooney which version of Turbo does your application depend on? As of 7.0.0-beta.8, There are tests in the suite dedicated to exercising that behavior: Along with fixture HTML that's very similar to the sample code you've shared in this issue:
<turbo-frame id="body-script" data-loaded-from="/src/tests/fixtures/frames/body_script_2.html">
<script>
if ("frameScriptEvaluationCount" in window) {
window.frameScriptEvaluationCount++
} else {
window.frameScriptEvaluationCount = 1
}
</script>
<a id="body-script-link" href="/src/tests/fixtures/frames/body_script.html">Load #body-script</a>
</turbo-frame> |
The application is using version I spent some time looking through If this is something we should change, let me know and I'll open a PR 👍 |
|
Investigating a little further, if I replace the chart in the view with a The above So I presume the issue is that the From what I can see in the HTML, the difference is that: During navigation, the It seems that form submission will not from remove the |
I've tried to reproduce the behavior you're describing in the test suite: diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html
index c308995..4544942 100644
--- a/src/tests/fixtures/frames.html
+++ b/src/tests/fixtures/frames.html
@@ -101,10 +101,20 @@
<turbo-frame id="body-script" target="body-script">
<a id="body-script-link" href="/src/tests/fixtures/frames/body_script.html">Load body-script</a>
+
+ <form id="body-script-form" method="post" action="/__turbo/redirect">
+ <input type="hidden" name="path" value="/src/tests/fixtures/frames/body_script.html">
+ <button>redirect to body-script</button>
+ </form>
</turbo-frame>
<turbo-frame id="eval-false-script" target="eval-false-script">
<a id="eval-false-script-link" href="/src/tests/fixtures/frames/eval_false_script.html">data-turbo-eval=false script</a></p>
+
+ <form id="eval-false-script-form" method="post" action="/__turbo/redirect">
+ <input type="hidden" name="path" value="/src/tests/fixtures/frames/eval_false_script.html">
+ <button>redirect to data-turbo-eval=false script</button>
+ </form>
</turbo-frame>
<turbo-frame id="recursive" recurse="composer" src="/src/tests/fixtures/frames/recursive.html">
diff --git a/src/tests/fixtures/frames/body_script.html b/src/tests/fixtures/frames/body_script.html
index 2d77614..1b91bf7 100644
--- a/src/tests/fixtures/frames/body_script.html
+++ b/src/tests/fixtures/frames/body_script.html
@@ -15,6 +15,11 @@
}
</script>
<a id="body-script-link" href="/src/tests/fixtures/frames/body_script_2.html">Load #body-script</a>
+
+ <form id="body-script-form" method="post" action="/__turbo/redirect">
+ <input type="hidden" name="path" value="/src/tests/fixtures/frames/body_script_2.html">
+ <button>redirect to body-script</button>
+ </form>
</turbo-frame>
</body>
</html>
diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts
index 4242d1f..f0ef338 100644
--- a/src/tests/functional/frame_tests.ts
+++ b/src/tests/functional/frame_tests.ts
@@ -393,7 +393,7 @@ test("test removing [disabled] attribute from eager-loaded frame navigates it",
await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
})
-test("test evaluates frame script elements on each render", async ({ page }) => {
+test("test evaluates frame script elements on each link navigation", async ({ page }) => {
assert.equal(await frameScriptEvaluationCount(page), undefined)
await page.click("#body-script-link")
@@ -403,12 +403,28 @@ test("test evaluates frame script elements on each render", async ({ page }) =>
assert.equal(await frameScriptEvaluationCount(page), 2)
})
-test("test does not evaluate data-turbo-eval=false scripts", async ({ page }) => {
+test("test evaluates frame script elements on each form submission", async ({ page }) => {
+ assert.equal(await frameScriptEvaluationCount(page), undefined)
+
+ await page.click("#body-script-form button")
+ assert.equal(await frameScriptEvaluationCount(page), 1)
+
+ await page.click("#body-script-form button")
+ assert.equal(await frameScriptEvaluationCount(page), 2)
+})
+
+test("test does not evaluate data-turbo-eval=false scripts during link navigation", async ({ page }) => {
await page.click("#eval-false-script-link")
await nextBeat()
assert.equal(await frameScriptEvaluationCount(page), undefined)
})
+test("test does not evaluate data-turbo-eval=false scripts during form submission", async ({ page }) => {
+ await page.click("#eval-false-script-form button")
+ await nextBeat()
+ assert.equal(await frameScriptEvaluationCount(page), undefined)
+})
+
test("test redirecting in a form is still navigatable after redirect", async ({ page }) => {
await page.click("#navigate-form-redirect")
await nextEventOnTarget(page, "form-redirect", "turbo:frame-load")The tests that I've added pass without any implementation changes.
If you're able to reproduce the behavior you're describing, and are able to write some failing tests, I think a PR would be great start. Thank you! |
In every non-trivial B2B application you’ll have to provide users with reports and charts to analyze data. Currently, Turbo does not evaluate
<script>tags in the response fromGETform submissions with the Accept turbo stream or html content type. TheGETform submission is preferred over aPOST, as the URL not changing can be problematic. For example, a user may wish to update the URL of the page to reflect the currently applied date range so that the user can copy/paste the URL to share a specific view of the report.It would be really useful for building reactive reports if we could "activate"
<script>elements immediately in responses from the server:The form above uses a
data-turbo-frameto target thesalesframe and tells Turbo to use the response from the form submission to replace the content of thesalesframe. The page URL is also updated by adding aturbo-actiondata attribute to the form that triggers the frame navigation.After submitting the form, the chart body appears but the
<script>function is not executed so the chart is not displayed.When loading this same view on the initial HTML request a canvas element is created with the actual chart rendered. So I presume the issue is that the
<script>is not being called after Turbo replaces the content of thesalesframe - but I don't know why.@seanpdoyle did you mention this was an anti-pattern in a comment somewhere? I was curious as to the correct Turbo way to approach this, as this seems like a great use case to compliment #449.
Options to work around this include using Turbo Streams with specific
GETrequests by replacingdata-turbo-framewithdata-turbo-stream="true"on the form, however, this appears to leave the URL unchanged as it's a stream action that doesn't involve a page visit. Moving the chart to a separateturbo_frame_tagalso evaluates and activates the<script>tag each time the form is submitted:The text was updated successfully, but these errors were encountered: