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

docs: v12 migration guide [LIBS-689] #884

Open
wants to merge 28 commits into
base: alpha
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8a874dc
docs: update js-to-jsx docs
KaiVandivier Oct 17, 2024
0fe32ec
docs: add known caveats to js-to-jsx
KaiVandivier Oct 17, 2024
1f3a32d
docs: add v12 migration guide
KaiVandivier Oct 17, 2024
16effc4
chore: deps
KaiVandivier Oct 17, 2024
7449f84
docs: add first draft of React 18 changes
KaiVandivier Oct 20, 2024
0d4d0bd
docs: add links and examples for React 18 test changes
KaiVandivier Oct 21, 2024
b123fd3
docs: improve link formatting
KaiVandivier Oct 21, 2024
b6784e1
docs: punctuation change
KaiVandivier Oct 21, 2024
ae82e2d
docs: more React 18 examples
KaiVandivier Oct 21, 2024
0dccdef
docs: space
KaiVandivier Oct 21, 2024
68f170c
docs: add some more react notes
KaiVandivier Oct 21, 2024
19ba4ab
docs: remove todo
KaiVandivier Oct 21, 2024
6ec8e95
docs: dashes
KaiVandivier Oct 29, 2024
bd7c4fa
docs: mention expected snapshot changes
KaiVandivier Oct 29, 2024
0d84728
docs: mention fireEvent to userEvent change
KaiVandivier Oct 29, 2024
9b839de
docs: comment on typescript changes
KaiVandivier Oct 29, 2024
1371274
docs: revision
KaiVandivier Oct 29, 2024
6cade34
docs: update breaking changes and deprecations
KaiVandivier Oct 30, 2024
725b14b
docs: comments about env vars
KaiVandivier Oct 30, 2024
ffdeb09
Merge branch 'alpha' into migration-guide-docs
KaiVandivier Oct 30, 2024
071f5dc
docs: intro revisions
KaiVandivier Nov 1, 2024
46ab715
docs: straight apostrophes
KaiVandivier Nov 1, 2024
271e359
docs: suggestions in React 18 section
KaiVandivier Nov 1, 2024
4178b36
docs: small revisions in react 18 section
KaiVandivier Nov 1, 2024
dbbc481
docs: env var suggestions
KaiVandivier Nov 1, 2024
c6364c3
docs: last suggestions
KaiVandivier Nov 2, 2024
9ecce0d
docs: remove "noticed in __ app" mentions
KaiVandivier Nov 2, 2024
03678d0
chore: formatting
KaiVandivier Nov 2, 2024
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
2 changes: 2 additions & 0 deletions docs/_sidebar.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
- [**Getting Started**](getting-started.md)
- [**Installation**](installation.md)
- [**Bootstrapping**](bootstrapping.md)
- [**Migration**](migration)
Copy link
Contributor

Choose a reason for hiding this comment

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

This sidebar isn't used, you need to add it to the sidebar in the developer portal!

So after merging this PR you'd need to merge the developer portal PR as well to get it to show up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, thanks for the reminder!

- [v12: Vite and React 18](migration/v12.md)
- [**Scripts**](scripts)
- [`d2-app-scripts init`](scripts/init.md)
- [`d2-app-scripts build`](scripts/build.md)
Expand Down
329 changes: 329 additions & 0 deletions docs/migration/v12.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,329 @@
# Platform v12 migration guide: Vite & React 18
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

Good news! Vite and React 18 in the app platform are ready to use!
Copy link
Contributor

Choose a reason for hiding this comment

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

This line, while good and happy, is very much "timed" as if for a blogpost or the announcement. If someone reads this guide in half a year or later it will be odd. Not sure if it can be omitted fully or reworded.

Paragraph below could use a little rewording as well to match this sentiment.


We’re very excited for these updates — both will significantly modernize the App Platform, and Vite will be a big upgrade from Create React App, which we used under the hood to build and serve apps previously. It will greatly improve the developer experience, and with greater control over the configuration, it will open up some powerful new possibilities for the platform.

Here are some tips about what to expect, and how to easily upgrade to the latest version of `@dhis2/cli-app-scripts` to take advantage of Vite and React 18.

### Notable changes

These are some things that you’ll see right away:
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

- It’s fast! Starting up an app is nearly instant, and building an app is about 3-4 times faster than before
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
- Plugins are handled better:
- Start-up of both the app and plugin is nearly instant
- The app and plugin are run on the same port
- Support for a plugin without an app is improved
- Code is shared between entrypoints, which makes bundles smaller
- HMR for code changes in the plugin will be as fast as in the app
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
- There are some new TypeScript features:
- Bootstrapping an app with a TypeScript template is supported: `d2-app-scripts init --typescript my-app`
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
- Vite has native support for TypeScript
- Although not in the App Platform package, with the latest `@dhis2/cli-style`, TypeScript type checking is performed when running `d2-style lint`
- With `@dhis2/ui` version 9, the UI library is now typed as well
- There’s a small suite of tools available in the CLI when running an app in dev mode:
- With the dev server running, press `h + enter` to see the options
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you link the vite docs about these tools?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, Vite doesn't have any docs for this 😢

- Options include exposing the server on LAN, opening the app in the browser, cleanly quitting the server, and restarting the dev server (which can be helpful if you’re modifying libraries in node_modules — see more below)
- The build output includes a summary to inspect chunks

### Future changes

With Vite, the door is open for some big future improvements:
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

- Extensible config: developers can add their own options to the Vite config, for example a plugin for Flow types, or to define import aliases
- Arbitrary entrypoints, beyond app/plugin/lib: Make a regular app, a configuration app, a capture plugin, a dashboard plugin, and more all from the same repo and sharing code between them

With respect to TypeScript, work is also under way to add types to data fetching tools using specs generated from OpenAPI.

## Getting started

By running these steps, you should be able to run your app right away:

1. `yarn add @dhis2/app-runtime @dhis2/ui -D @dhis2/cli-app-scripts`
2. `npx yarn-deduplicate yarn.lock && yarn`
3. Try out `yarn start --allowJsxInJs`, and your app should be running 🚀

There will be some other changes you will probably want to make, which are described in the following sections. Our goal is to make it easy to adopt the new changes, so we have some tools to facilitate the process.

## Vite & JSX

As suggested by the `--allowJsxInJs` flag, Vite does not allow JSX syntax in files without a .jsx or .tsx extension by default; attempting to run `start` or `build` in that case will throw an error like `✘ [ERROR] The JSX syntax extension is not currently enabled`.

JSX in .js files is not supported for performance reasons: it saves needing to parse non-JSX files for JSX syntax. We can add complicated config to enable parsing of JSX in .js files, but that approach has several downsides:

1. It’s nonstandard — it is most correct to use the right file extensions
2. It requires more config to maintain
3. It has performance costs on startup and when parsing files
The new default for `@dhis2/cli-app-scripts`, therefore, will be the same as Vite’s. To make the transition easier, though, we provide some tools to 1) support JSX in .js files temporarily, and 2) easily migrate JS files to .jsx extensions if they contain JSX syntax.

### Allow JSX in JS temporarily

To make it as fast as possible to upgrade and get up and running with Vite and React 18, we added an `--allowJsxInJs` flag that you can pass to `start` and `build` scripts, so that you can test out the changes without any file renaming. In many cases, you should be able to upgrade your platform dependencies and run `start` with the flag and see your app running! Other times, you may need to add a few tweaks — see the troubleshooting and breaking changes sections below.

Due to the downsides mentioned above, however, we will remove the `--allowJsxInJs` option in the next major version. When you are ready to rename your files, you can use the migration script we’ve made.

### .js to .jsx migration script

We made a codemod that can migrate your codebase to the right file extensions quickly! It checks .js files for JSX syntax, updates the extension if needed, then updates imports within the file tree to target the new filenames.

```sh
yarn d2-app-scripts migrate js-to-jsx
```

Check out the [script docs](../scripts/migrate/js-to-jsx.md) for options, tips, and caveats.

## React 18

In terms of the React 18-related changes, upgrading to `@dhis2/cli-app-scripts` version 12 should generally a smooth process for the consuming apps. Most of the [changes](https://react.dev/blog/2022/03/08/react-18-upgrade-guide) shouldn't affect most apps.
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

### Default props

The main change in React 18 that is expected to affect apps at runtime is that using `MyComponent.defaultProps = { ... }` is deprecated, and will be removed in the next major version. While non-breaking, this change comes along with warning messages in the console for every rendered component that uses them, which can be chatty and annoying.

The fix is to move prop defaults -- either to default parameters for functional components, or to a static property for class components. This code mod can help with the migration, and was used for the `@dhis2/ui` library: [transform.js](https://gist.github.com/kabaros/45ed16660ac89dc48d22e03836b1e981)

**NB!** Note that default params for functional components are computed on each render, so default values like arrays and objects as defaults may unintentionally retrigger `useEffect` and `useMemo` functions on each render. Instead, use a stable reference like so:
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

```js
const arr = []
const MyComponent = ({ options = arr }) => {
/* ... */
}
```

KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
### Tests

React 18 has minimal effects on the operation of an app in a browser, but it has a more significant effect on tests, depending on what tools are used and how the tests are written.

#### Snapshots

When the tests are up and running, you can expect some small changes from React 18. You may see changed classnames and different whitespace; these shouldn't cause any functional changes, and they're safe to approve.

#### React Testing Library

Testing-library supports React 18 since v13, so the first step is to upgrade that library and the supporting libraries around it. These major versions bumps come with a few breaking changes that we will document here with some examples to make migration easier.

Examples come from the migration for the Aggregate Data Entry app; you can check out the PR [here](https://github.com/dhis2/aggregate-data-entry-app/pull/396) for more examples in context.

##### `userEvent`

`userEvent` is async now in `@testing-library/user-event`. Check the [release notes](https://github.com/testing-library/user-event/releases/tag/v14.0.0) for an idea of the improvements and new API. In practice, this means mostly that you'd need to add an `await` for `userEvent` interactions like `userEvent.click()`.

Actions using the `fireEvent` API should also be migrated to `userEvent`, e.g. `fireEvent.change(selector(), { target: { value: 'input text' } })` should be changed to `userEvent.type(selector(), 'input text')`.

Here are some examples that illustrate the changes:

```diff
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll need to do a little testing to see if this will render properly on the developer portal

+ import userEvent from '@testing-library/user-event'

- it('should allow re-running validation', () => {
+ // Make the function async:
+ it('should allow re-running validation', async () => {
// ...

- userEvent.click(getByText('Run validation again'))
- // Or this other form:
- fireEvent.click(getByText('Run validation again'))
+ // Now that it's asynchronous, await the event:
+ await userEvent.click(getByText('Run validation again'))

await findByText('2 medium priority alerts')
expect(queryByText('There was a problem running validation')).toBeNull()
})
```
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

```diff
- const login = async ({ user }) => {
+ // Don't need `user`; will use `userEvent` later:
+ const login = async () => {
- // Remove fireEvent calls:
- fireEvent.change(screen.getByLabelText('Username'), {
- target: { value: '[email protected]' },
- })
- fireEvent.change(screen.getByLabelText('Password'), {
- target: { value: 'SolanOgLudvig' },
- })
+ // Replace with userEvent.type():
+ await userEvent.type(screen.getByLabelText('Username'), '[email protected]')
+ await userEvent.type(screen.getByLabelText('Password'), 'SolanOgLudvig')

- await user.click(screen.getByRole('button', { name: /log in/i }))
+ // Replace `user` with `userEvent`:
+ await userEvent.click(screen.getByRole('button', { name: /log in/i }))
}
```

##### `renderHook`

`renderHook` is not a separate library anymore; it is part of the `@testing-library/react`, and it has [quite a different API](https://testing-library.com/docs/react-testing-library/api/#renderhook). (NB: Some docs online still point to the old version, which can be confusing.)
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

- Use `import { renderHook } from '@testing-library/react'` instead of `import { renderHook } from '@testing-library/react-hooks`
- The [return type](https://testing-library.com/docs/react-testing-library/api/#renderhook-result) for renderHook is different. One major difference is the absence of `waitForNextUpdate` returned from `renderHook`. Now it can be replaced with the `waitFor` from `@testing-library`, and adding an expectation to wait for, for example.
- Testing hooks that throw errors is different. Use `expect(renderHook(() => { ... })).toThrow(....)`

This example demonstrates some of the changes:

```diff
- import { renderHook } from '@testing-library/react-hooks'
+ // Update import, and include waitFor:
+ import { renderHook, waitFor } from '@testing-library/react'
import React from 'react'
import { useRootOrgData } from './use-root-org-data.js'

it('should provide the org unit data', async () => {
- const { result, waitForNextUpdate } = renderHook(
- () => useRootOrgData(['A0000000000']),
- { wrapper }
- )
+ // waitForNextUpdate is no longer part of the result:
+ const { result } = renderHook(() => useRootOrgData(['A0000000000']), {
+ wrapper,
+ })

- await waitForNextUpdate()
+ // Instead, use waitFor:
+ await waitFor(() => {})
// ...
})
```

In some cases, `renderHookResult.waitFor` can just be removed and doesn't need replacing:

```diff
it('should update validationToRefresh', async () => {
- const { result, waitFor } = renderHook(useValidationStore)
+ const { result } = renderHook(useValidationStore)

act(() => {
result.current.setValidationToRefresh(true)
})

- await waitFor(() => {
- expect(result.current.getValidationToRefresh()).toBe(true)
- })
+ expect(result.current.getValidationToRefresh()).toBe(true)
})
```

And testing errors:

```diff
it('throws an error when passing both a client- and a serverDate', async () => {
const clientDate = new Date('2022-10-13 10:00:00')
const serverDate = new Date('2022-10-13 08:00:00')
- const { result } = renderHook(() =>
- useClientServerDate({ clientDate, serverDate })
- )
- expect(result.error).toEqual(
- new Error(
- '`useClientServerDate` does not accept both a client and a server date'
- )

+ expect(() => {
+ renderHook(() => useClientServerDate({ clientDate, serverDate }))
+ }).toThrow(
+ '`useClientServerDate` does not accept both a client and a server date'
+ )
})
```

##### Fake timers

Faking timers can help with tests that seem to fail with concurrency issues:

```diff
describe('<Comment />', () => {
+ // Set up fake timers:
+ beforeEach(() => {
+ jest.useFakeTimers
+ })

afterEach(() => {
// ...
})

it('shows a loading indicator when submitting a comment change', async () => {
// ...

- const { getByRole, queryByRole } = render(<Comment item={item} />)
+ const { getByRole, queryByRole, findByRole } = render(
+ <Comment item={item} />
+ )

- userEvent.click(getByRole('button', { name: 'Edit comment' }))
+ const editButton = await findByRole('button', { name: 'Edit comment' })
+ // Set up user event with fake timers:
+ const user = userEvent.setup({
+ advanceTimers: jest.advanceTimersByTime,
+ })
+ await user.click(editButton)

// ...

expect(queryByRole('progressbar')).not.toBeInTheDocument()
- userEvent.click(getByRole('button', { name: 'Save comment' }))
- expect(getByRole('progressbar')).toBeInTheDocument()
+ // Use the setup from above:
+ const btnSaveComment = await findByRole('button', {
+ name: 'Save comment',
+ })
+ await user.click(btnSaveComment)
+ await findByRole('progressbar')
})
})
```

#### Enzyme

Enzyme is [considered dead](https://dev.to/wojtekmaj/enzyme-is-dead-now-what-ekl): there is no official adapter for React 18 (or 17), so it's best to [migrate to React Testing Library](https://testing-library.com/docs/react-testing-library/migrate-from-enzyme/) for tests, if possible.

If it's not currently possible to migrate, there is an [unofficial React 18 adapter](https://github.com/cfaester/enzyme-adapter-react-18) that can help in the mean time.
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

## Environment variables

### `REACT_APP_` prefix no longer needed

Now that we're moving away from Create React App, the formatting for environment variable names has gotten simpler.
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

As before, environment variables with names that start with `DHIS2_` are added to the app, and can be picked up from `.env` files or on the command line for example. Before, the platform had to add `REACT_APP_` before `DHIS2_` so that Create React App would add the variable to the app, so a variable like `DHIS2_MY_VAR` in the environment would become accessible on `process.env.REACT_APP_DHIS2_MY_VAR` in the app. Now, the `REACT_APP_` prefix isn't needed, so the variable can be accessed on just `process.env.DHIS2_MY_VAR`.

The `REACT_APP_`-prefixed variables are now deprecated, but they will be kept around alongside the shorter `DHIS2_` variables in this version for backwards compatibility. For example, both `process.env.REACT_APP_DHIS2_MY_VAR` and `process.env.DHIS2_MY_VAR` will be available. The `REACT_APP_` prefixed variables will be removed in the next version though, so make sure to migrate to just the shorter `DHIS2_` variable names.
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

### `import.meta.env`

By default, Vite adds its environment variables to the [`import.meta.env` object](https://vite.dev/guide/env-and-mode#env-variables). The App Platform will add `DHIS2_` env vars to `import.meta.env` as well, and the configured [`envPrefix`](https://vite.dev/config/shared-options.html#envprefix) is `DHIS2_`. Emphasis is given to environment variables on the `process.env` object, however, since it is a more widely-used pattern.

## Troubleshooting & tips

Here are some edge cases that were encountered when upgrading some apps with the new platform version:

- There is a bug in Vite: importing from a directory, e.g. `‘./app/‘` where we would assume to get `./app/index.[jt]s`, can incorrectly resolve to a file with a similar name to the directory, e.g. `./App.tsx`. I see this as a bug with Vite, but as a workaround, file and dir names can be changed so they don’t conflict. Noticed in the Maintenance app beta.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the Maintenance app example be linked/explained?

- If you’re using `identity-obj-proxy` in your test config, make sure you add it to the project’s dependencies. Previously, some apps got away with using the proxy without an explicitid dependency because the package is a dependency of `react-scripts`
- You may see some build warnings coming from the `mathjs` package. Upgrading to a recent version of `mathjs` will handle these warnings. Noticed in analytics app (the analytics library has a dependency on `mathjs`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the mention of the Analytics app matter? It might confuse more than solve as the solution is a simple upgrade of a package. Maybe add the command to handle this upgrade instead?


## Full list of breaking changes and deprecations

Larger items are mentioned above, but these lists are included for completeness.
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved

### Breaking changes

1. Node 18 || 20+ is required
KaiVandivier marked this conversation as resolved.
Show resolved Hide resolved
2. JSX in files without .jsx or .tsx extensions is not supported by default
3. React 18: Most significant testing changes are described above; the rest can be read about in the React [migration guide](https://react.dev/blog/2022/03/08/react-18-upgrade-guide)
4. Flow types won’t work, although this is a motivator for extensible config
5. Import aliases will likely break (also a motivator for extensible config)
6. `global.variableName` is no longer supported; use `window.variableName` instead
7. In order to make the dev server available on LAN, the `--host` flag needs to be passed to the start script
8. Workers imported in the “webpack” way will need to be imported in a new way: [docs](https://vitejs.dev/guide/features.html#web-workers)
9. An `App.jsx` entrypoint is no longer quietly added if a plugin entrypoint is defined or for a defined but empty entrypoints object
10. Plugins and apps are started together in the same process on the same port
11. Custom `index.html` files are no longer supported
12. `import * as MyClass from ‘my-module’` is now more restrictive, and may break in some cases. So far, changing the import to `import MyClass from ‘my-module’` has fixed it. DOMPurify is one example case, noticed in the Login App
Copy link
Contributor

Choose a reason for hiding this comment

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

Can Login App example be linked/explained?


### Deprecations

- The default `direction` config value is `ltr`; in the future, it will be `auto`
- `--allowJsxInJs` will be removed
Copy link
Contributor

Choose a reason for hiding this comment

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

wil this be removed in the underlying Vite CLI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure which bullet point you mean haha, but here's an answer for each:

  1. Env vars are under our control, but I'm actually rethinking this point, i.e. maybe we should keep variables on process.env -- Earlier, I thought import.meta.env was a pretty standard thing with ES6 and that we should migrate to that for env vars, but I think that's actually just a Vite pattern, so it might be "more standard" to keep env vars on process.env. If we do that, we can at least deprecate the REACT_APP_ prefixes for the variable keys
  2. (Probably obvious, but direction is also under our control)
  3. For --allowJsxInJs, that's also under our control -- it's a flag that our own CLI reads, and adds a bunch of kinda complex Vite config in makeViteConfig.mjs. That additional config may or may not be supported in future versions of Vite, but I think we should remove our --allowJsxInJs flag and config in the next cli-app-scripts version to make things simpler, standardized, and more performant

- Some deprecated PWA config caching option names will be removed in the next version
- Env vars prefixed with `REACT_APP_DHIS2_` will be removed in the next version in favor of just the `DHIS2_` prefix
Loading
Loading