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

Replace postForm() with postFormData() #62

Merged
merged 1 commit into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions strcalc/src/main/frontend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand All @@ -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
1 change: 0 additions & 1 deletion strcalc/src/main/frontend/components/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion strcalc/src/main/frontend/components/calculator.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
12 changes: 8 additions & 4 deletions strcalc/src/main/frontend/components/calculator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
30 changes: 17 additions & 13 deletions strcalc/src/main/frontend/components/calculator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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'))
})
})
10 changes: 7 additions & 3 deletions strcalc/src/main/frontend/components/calculators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
25 changes: 25 additions & 0 deletions strcalc/src/main/frontend/components/calculators.test.js
Original file line number Diff line number Diff line change
@@ -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' }))
})
})
7 changes: 4 additions & 3 deletions strcalc/src/main/frontend/components/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}

/**
Expand Down
41 changes: 7 additions & 34 deletions strcalc/src/main/frontend/components/request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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))
})
})
})
2 changes: 1 addition & 1 deletion strcalc/src/main/frontend/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
)
2 changes: 1 addition & 1 deletion strcalc/src/main/frontend/main.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
30 changes: 30 additions & 0 deletions strcalc/src/main/frontend/test/fetch-stub.js
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down