Skip to content

Commit

Permalink
fix(crwa): Explicit check for possible null value in `entry.client.ts…
Browse files Browse the repository at this point in the history
…x` (redwoodjs#9251)

**Problem**
Fixes redwoodjs#9059 which is a type error raised when you have strict type
checking enabled. The `getElementById` call can return null as the [MDN
docs](https://developer.mozilla.org/en-US/docs/Web/API/Document/getElementById#return_value)
mention.

**Changes**
Adds a basic null check taken from the solution given in redwoodjs#9059. 

**Considerations**
This adds some boilerplate code to a user facing file but I think it's a
reasonable addition. It's readable and clear what the code is doing
without the need for further explanation or technical knowledge. In the
case where this causes a runtime error then the message is clearer:
![Screenshot from 2023-10-02
23-16-38](https://github.com/redwoodjs/redwood/assets/56300765/ca6d51bd-2c28-438e-87d3-7411e54721ec)

I haven't immediately been able to think of some hidden solution to this
that isn't exposed to the user in some boilerplate code.
  • Loading branch information
Josh-Walker-GM authored Oct 10, 2023
1 parent 229bd4b commit b0964a9
Show file tree
Hide file tree
Showing 15 changed files with 341 additions and 0 deletions.
6 changes: 6 additions & 0 deletions __fixtures__/test-project/web/src/entry.client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import App from './App'
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Entry Client Null Check


**Description**

When you enable typescript strict mode the return type of the `getElementById` function will be `HTMLElement | null`. This means that you need to check if the element exists before using it. This codemod adds a check to ensure the element exists before using it.

You will also see a clearer error message in the browser console if the element does not exist.

**Examples**

For example the following `entry.client.tsx`:
```tsx
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}

```
would become:
```tsx
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}

```
where a check to ensure the element exists has been added.

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

// Some user is already checking for null
if(redwoodAppElement === null){
throw new Error(
"Opps I must have changed the div name or deleted the div completely!"
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

// Some user is already checking for null
if(redwoodAppElement === null){
throw new Error(
"Opps I must have changed the div name or deleted the div completely!"
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

// Some random additional user code
if(Math.random() > 0.5){
console.log("Some random code execution...")
}else{
console.log("There are", redwoodAppElement.children?.length, "div children in the redwood-app element.")
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

// Some random additional user code
if(Math.random() > 0.5){
console.log("Some random code execution...")
}else{
console.log("There are", redwoodAppElement.children?.length, "div children in the redwood-app element.")
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
const root = createRoot(redwoodAppElement)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const hahaRenamedVariable = document.getElementById('redwood-app-custom-div-id')

if (hahaRenamedVariable.children?.length > 0 || Math.random() > 0.5) {
hydrateRoot(hahaRenamedVariable, <App />)
} else {
const root = createRoot(hahaRenamedVariable)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { hydrateRoot, createRoot } from 'react-dom/client'

import App from './App'
/**
* When `#redwood-app` isn't empty then it's very likely that you're using
* prerendering. So React attaches event listeners to the existing markup
* rather than replacing it.
* https://reactjs.org/docs/react-dom-client.html#hydrateroot
*/
const hahaRenamedVariable = document.getElementById('redwood-app-custom-div-id')

if (hahaRenamedVariable.children?.length > 0 || Math.random() > 0.5) {
hydrateRoot(hahaRenamedVariable, <App />)
} else {
const root = createRoot(hahaRenamedVariable)
root.render(<App />)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
describe('entryClientNullCheck', () => {
it('Handles the default case', async () => {
await matchTransformSnapshot('entryClientNullCheck', 'default')
})

it('User has already implemented the check', async () => {
await matchTransformSnapshot('entryClientNullCheck', 'alreadyChecking')
})

it('Additional code present', async () => {
await matchTransformSnapshot('entryClientNullCheck', 'moreCode')
})

it('Unintelligible changes to entry file', async () => {
await matchTransformSnapshot('entryClientNullCheck', 'unintelligible')
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import type { FileInfo, API } from 'jscodeshift'

export default function transform(file: FileInfo, api: API) {
const j = api.jscodeshift
const ast = j(file.source)

// Get the expected variable declaration
const node = ast.find(j.VariableDeclaration, {
declarations: [{ id: { name: 'redwoodAppElement' } }],
})

// If it doesn't exist, bail out and let the user know
if (node.length === 0) {
console.warn(
"\nCould not find 'redwoodAppElement' variable declaration. Please make the necessary changes to your 'web/src/index.js' file manually.\n"
)
return file.source
}

// Insert the new null check
node.insertAfter(
j.ifStatement(
j.unaryExpression('!', j.identifier('redwoodAppElement')),
j.blockStatement([
j.throwStatement(
j.newExpression(j.identifier('Error'), [
j.literal(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
),
])
),
])
)
)

return ast.toSource()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import path from 'path'

import fg from 'fast-glob'
import type { TaskInnerAPI } from 'tasuku'
import task from 'tasuku'

import { getPaths } from '@redwoodjs/project-config'

import runTransform from '../../../lib/runTransform'

export const command = 'entry-client-null-check'
export const description = '(v6.x.x->v6.x.x) Converts world to bazinga'

export const handler = () => {
task('Entry Client Null Check', async ({ setOutput }: TaskInnerAPI) => {
await runTransform({
transformPath: path.join(__dirname, 'entryClientNullCheck.js'),
targetPaths: fg.sync('entry.client.{jsx,tsx}', {
cwd: getPaths().web.src,
absolute: true,
}),
})

setOutput('All done! Run `yarn rw lint --fix` to prettify your code')
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import App from './App'
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import App from './App'
*/
const redwoodAppElement = document.getElementById('redwood-app')

if (!redwoodAppElement) {
throw new Error(
"Could not find an element with ID 'redwood-app'. Please ensure it exists in your 'web/src/index.html' file."
)
}

if (redwoodAppElement.children?.length > 0) {
hydrateRoot(redwoodAppElement, <App />)
} else {
Expand Down

0 comments on commit b0964a9

Please sign in to comment.