Merge pull request #62 from mbland/replace-postform-with-postformdata

Replace postForm() with postFormData()

strcalc/src/main/frontend/README.md

@@ -178,6 +178,30 @@ You'll need to be careful not to commit these changes. You can `git stash` them
 or run `git restore package.json`, followed by `pnpm i` to match the CI build
 again.
 
+## Notes
+
+Some implementation notes I may want to work into documentation or blog
+posts one day.
+
+### Document vs. DocumentFragment and <form> behavior
+
+Originally I tried using [DocumentFragment][] objects in the tests as much
+as I could to avoid polluting the main [Document][]. However, differences in
+[<form>][] behavior led me to implement a different scheme, adding and
+removing unique [<div>][]s to and from the main Document instead.
+
+Specifically, `form.action` resolves differently depending on how it's created.
+Elements in a DocumentFragment are in a separate DOM, causing the <form
+action="/fetch"> attribute to be:
+
+- '/fetch' in jsdom
+- '' in Chrome
+- `${document.location.href}/fetch` in Firefox
+
+Creating a <form> element via `document.createElement()` causes
+`form.action` to become `${document.location.href}/fetch` in every
+environment.
+
 [mbland/tomcat-servlet-testing-example]: https://github.com/mbland/tomcat-servlet-testing-example
 [top level README.md]: ../../../../README.md
 [Node.js]: https://nodejs.org/
@@ -204,3 +228,7 @@ again.
 [PowerShell]: https://learn.microsoft.com/powershell/
 [`start`]: https://learn.microsoft.com/windows-server/administration/windows-commands/start
 [eslint-plugin-jsdoc]: https://www.npmjs.com/package/eslint-plugin-jsdoc
+[DocumentFragment]: https://developer.mozilla.org/docs/Web/API/DocumentFragment
+[Document]: https://developer.mozilla.org/docs/Web/API/Document
+[<form>]: https://developer.mozilla.org/docs/Web/HTML/Element/form
+[<div>]: https://developer.mozilla.org/docs/Web/HTML/Element/div

strcalc/src/main/frontend/components/app.js

@@ -21,7 +21,6 @@ export default class App {
    * demonstrate how to design much larger applications for testability.
    * @param {object} params - parameters made available to all initializers
    * @param {Element} params.appElem - parent Element containing all components
-   * @param {string} params.apiUrl - API backend server URL
    * @param {object} params.calculators - calculator implementations
    */
   init(params) {

strcalc/src/main/frontend/components/calculator.hbs

@@ -3,7 +3,7 @@
   License, v. 2.0. If a copy of the MPL was not distributed with this
   file, You can obtain one at https://mozilla.org/MPL/2.0/.
 --}}
-<form action="{{ apiUrl }}" method="post">
+<form>
   <label for="numbers">Enter the string of numbers to add:</label>
   <input type="text" name="numbers" id="numbers" required />
   <fieldset class="impl">

strcalc/src/main/frontend/components/calculator.js

@@ -11,13 +11,12 @@ export default class Calculator {
    * Initializes the Calculator within the document.
    * @param {object} params - parameters made available to all initializers
    * @param {Element} params.appElem - parent Element containing all components
-   * @param {string} params.apiUrl - API backend server URL
    * @param {object} params.calculators - calculator implementations
    */
-  init({ appElem, apiUrl, calculators }) {
+  init({ appElem, calculators }) {
     const calcOptions = Object.entries(calculators)
       .map(([k, v]) => ({ value: k, label: v.label }))
-    const t = Template({ apiUrl, calcOptions })
+    const t = Template({ calcOptions })
     const [ form, resultElem ] = t.children
 
     appElem.appendChild(t)
@@ -32,11 +31,16 @@ export default class Calculator {
     event.preventDefault()
 
     const form = event.currentTarget
+    const data = new FormData(form)
     const selected = form.querySelector('input[name="impl"]:checked').value
     const result = resultElem.querySelector('p')
 
+    // None of the backends need the 'impl' parameter, and the Java backend
+    // will return a 500 if we send it.
+    data.delete('impl')
+
     try {
-      const response = await calculators[selected].impl(form)
+      const response = await calculators[selected].impl(data)
       result.textContent = `Result: ${response.result}`
     } catch (err) {
       result.textContent = err

strcalc/src/main/frontend/components/calculator.test.js

@@ -7,24 +7,27 @@
 
 import Calculator from './calculator'
 import { afterAll, afterEach, describe, expect, test, vi } from 'vitest'
-import { resolvedUrl } from '../test/helpers'
 import StringCalculatorPage from '../test/page'
 
 // @vitest-environment jsdom
 describe('Calculator', () => {
   const page = StringCalculatorPage.new()
 
   const setup = () => {
-    const postForm = vi.fn()
+    const postFormData = vi.fn()
     const calculators = {
-      'api': { label: 'API', impl: postForm },
+      'api': { label: 'API', impl: postFormData },
       'browser': { label: 'Browser', impl: () => {} }
     }
 
-    new Calculator().init({
-      appElem: page.appElem, apiUrl: './add', calculators
-    })
-    return { page, postForm }
+    new Calculator().init({ appElem: page.appElem, calculators })
+    return { page, postFormData }
+  }
+
+  const expectedFormData = (numbersString) => {
+    const data = new FormData()
+    data.append('numbers', numbersString)
+    return data
   }
 
   afterEach(() => page.clear())
@@ -34,25 +37,26 @@ describe('Calculator', () => {
     const { page } = setup()
 
     expect(page.form()).not.toBeNull()
-    expect(page.form().action).toBe(resolvedUrl('./add'))
+    expect(page.result()).not.toBeNull()
   })
 
   test('updates result placeholder with successful result', async () => {
-    const { page, postForm } = setup()
-    postForm.mockResolvedValueOnce({ result: 5 })
+    const { page, postFormData } = setup()
+    postFormData.mockResolvedValueOnce({ result: 5 })
 
     const result = vi.waitFor(page.enterValueAndSubmit('2,2'))
 
     await expect(result).resolves.toBe('Result: 5')
-    expect(postForm).toHaveBeenCalledWith(page.form())
+    expect(postFormData).toHaveBeenCalledWith(expectedFormData('2,2'))
   })
 
   test('updates result placeholder with error message', async () => {
-    const { page, postForm } = setup()
-    postForm.mockRejectedValueOnce(new Error('D\'oh!'))
+    const { page, postFormData } = setup()
+    postFormData.mockRejectedValueOnce(new Error('D\'oh!'))
 
     const result = vi.waitFor(page.enterValueAndSubmit('2,2'))
 
     await expect(result).resolves.toBe('Error: D\'oh!')
+    expect(postFormData).toHaveBeenCalledWith(expectedFormData('2,2'))
   })
 })

strcalc/src/main/frontend/components/calculators.js

@@ -4,12 +4,16 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-import { postForm } from './request'
+import { postFormData } from './request'
+
+export const DEFAULT_ENDPOINT = './add'
+
+const defaultPost = async (data)=> postFormData(DEFAULT_ENDPOINT, data)
 
 /**
  * Collection of production String Calculator implementations
  */
 export default {
-  'api': { label: 'Tomcat backend API (Java)', impl: postForm },
-  'browser': { label: 'In-browser (JavaScript)', impl: postForm }
+  'api': { label: 'Tomcat backend API (Java)', impl: defaultPost },
+  'browser': { label: 'In-browser (JavaScript)', impl: defaultPost }
 }

strcalc/src/main/frontend/components/calculators.test.js

@@ -0,0 +1,25 @@
+/* eslint-env browser, node, jest, vitest */
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+import { default as calculators, DEFAULT_ENDPOINT } from './calculators'
+import { afterEach, describe, expect, test, vi } from 'vitest'
+import setupFetchStub from '../test/fetch-stub'
+import { postOptions } from './request'
+
+describe('calculators', () => {
+  afterEach(() => { vi.unstubAllGlobals() })
+
+  test('defaultPost requests expected backend', async () => {
+    const data = new FormData()
+    const fetchStub = setupFetchStub(JSON.stringify({ ok: true }))
+    data.append('want', 'status')
+
+    await expect(calculators.api.impl(data)).resolves.toEqual({ ok: true })
+    expect(fetchStub).toHaveBeenCalledWith(
+      DEFAULT_ENDPOINT, postOptions({ want: 'status' }))
+  })
+})

strcalc/src/main/frontend/components/request.js

@@ -7,11 +7,12 @@
 /**
  * Posts the data from a <form> via fetch() and returns the response object
  * @see https://simonplend.com/how-to-use-fetch-to-post-form-data-as-json-to-your-api/
- * @param {FormData} form - form containing data to POST
+ * @param {string} url - address of server request
+ * @param {FormData} formData - data to include in the POST request
  * @returns {Promise<any>} - response from the server
  */
-export async function postForm(form) {
-  return post(form.action, Object.fromEntries(new FormData(form).entries()))
+export async function postFormData(url, formData) {
+  return post(url, Object.fromEntries(formData.entries()))
 }
 
 /**

strcalc/src/main/frontend/components/request.test.js

@@ -4,22 +4,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
-import { post, postForm, postOptions } from './request'
+import { post, postFormData, postOptions } from './request'
 import { afterEach, describe, expect, test, vi } from 'vitest'
-import { resolvedUrl } from '../test/helpers'
+import setupFetchStub from '../test/fetch-stub'
 
 // @vitest-environment jsdom
 describe('Request', () => {
   const req = { want: 'foo' }
 
-  const setupFetchStub = (body, options) => {
-    const fetchStub = vi.fn()
-
-    fetchStub.mockReturnValueOnce(Promise.resolve(new Response(body, options)))
-    vi.stubGlobal('fetch', fetchStub)
-    return fetchStub
-  }
-
   afterEach(() => { vi.unstubAllGlobals() })
 
   describe('post', () => {
@@ -53,34 +45,15 @@ describe('Request', () => {
     })
   })
 
-  describe('postForm', () => {
+  describe('postFormData', () => {
     test('succeeds', async () => {
-      // We have to be careful creating the <form>, because form.action resolves
-      // differently depending on how we created it.
-      //
-      // Originally I tried creating it using fragment() from '../test/helpers',
-      // which creates elements using a new <template> containing a
-      // DocumentFragment. However, the elements in that DocumentFragment are in
-      // a separate DOM. This caused the <form action="/fetch"> attribute to be:
-      //
-      // - '/fetch' in jsdom
-      // - '' in Chrome
-      // - `#{document.location.origin}/fetch` in Firefox
-      //
-      // Creating a <form> element via document.createElement() as below
-      // causes form.action to become `#{document.location.origin}/fetch` in
-      // every environment.
-      const form = document.createElement('form')
-      const resolvedAction = resolvedUrl('./fetch')
+      const fd = new FormData()
       const res = { foo: 'bar' }
       const fetchStub = setupFetchStub(JSON.stringify(res))
+      Object.entries(req).forEach(([k,v]) => fd.append(k,v))
 
-      form.action = '/fetch'
-      form.innerHTML = '<input type="text" name="want" id="want" value="foo" />'
-
-      expect(form.action).toBe(resolvedAction)
-      await expect(postForm(form)).resolves.toEqual(res)
-      expect(fetchStub).toHaveBeenCalledWith(resolvedAction, postOptions(req))
+      await expect(postFormData('/fetch', fd)).resolves.toEqual(res)
+      expect(fetchStub).toHaveBeenCalledWith('/fetch', postOptions(req))
     })
   })
 })

strcalc/src/main/frontend/main.js

@@ -30,7 +30,7 @@ document.addEventListener(
   'DOMContentLoaded',
   () => {
     const appElem = document.querySelector('#app')
-    new App().init({ appElem, apiUrl: './add', calculators })
+    new App().init({ appElem, calculators })
   },
   { once: true }
 )

strcalc/src/main/frontend/main.test.js

@@ -5,7 +5,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 import { describe, afterEach, expect, test } from 'vitest'
-import { PageLoader } from './test/helpers'
+import { PageLoader } from './test/page-loader.js'
 import StringCalculatorPage from './test/page'
 
 describe('String Calculator UI on initial page load', () => {

strcalc/src/main/frontend/test/fetch-stub.js

@@ -0,0 +1,30 @@
+/* eslint-env browser, node */
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+import {vi} from 'vitest'
+
+/**
+ * Stubs the global fetch() with a vi.fn() configured with a Response
+ *
+ * Use `afterEach(() => { vi.unstubAllGlobals() })` to clean up this stub
+ * after every test.
+ * @see https://developer.mozilla.org/docs/Web/API/Fetch_API/Using_Fetch
+ * @see https://developer.mozilla.org/docs/Web/API/Response/Response
+ * @param {object} body - an object defining a body for the response
+ * @param {object} options - optional values to include in the response
+ * @param {object} options.status - HTTP status code of the response
+ * @param {object} options.statusText - text accompanying the status response
+ * @param {object} options.headers - HTTP Headers to include with the response
+ * @returns {Function} - a vi.fn() stub configured to return a Response once
+ */
+export default function setupFetchStub(body, options) {
+  const fetchStub = vi.fn()
+
+  fetchStub.mockReturnValueOnce(Promise.resolve(new Response(body, options)))
+  vi.stubGlobal('fetch', fetchStub)
+  return fetchStub
+}

strcalc/src/main/frontend/test/helpers.js → strcalc/src/main/frontend/test/page-loader.js

@@ -10,15 +10,6 @@
  * @module test-helpers
  */
 
-/**
- * Resolves URL path against the current document location
- * @param {string} path - path to resolve
- * @returns {string} - the result of resolving path against document.location
- */
-export function resolvedUrl(path) {
-  return new URL(path, document.location.href).toString()
-}
-
 /**
  * Enables tests to load page URLs both in the browser and in Node using JSDom.
  */