Record decisions for update to the repository organisation ahead of v5

decision-records/005-repository-organisation-for-v5.md

@@ -0,0 +1,121 @@
+# Decision Record Title
+
+Update our repository structure to prepare v5
+<!-- The title should be a short present tense imperative phrase, less than 50
+     characters, like a git commit message. -->
+
+## Decision
+
+<!-- An overview of the decision made. -->
+
+To prepare our work for v5, we've agreed to make the following changes to the files in our repository and their location:
+
+1. Move the sources (currently `src` ) and built package files (currently `package`) under the same npm package. We'll keep shipping only the built files to npm for now, excluding the source files from the package (either via `.npmignore` or using the `files` field of `package.json`)
+2. Remove the built package files (currently `package`) from the versioning
+3. Update our release process to make sure we keep checking the changes (possibly manually) between the package to be released and the previous version
+4. Leave `dist` in its current location
+
+This will yield the following structure for our repository:
+
+```
+govuk-frontend // The root of our repository
+├── ... // Irrelevant files of folder omitted for brevity
+├── dist // Not touching the location, naming might change for clarity
+├── package // Naming might change for clarity
+│   ├── src // Sources are now here, in version control, not shipped to `npm`
+│   │   └── govuk // Structure inside `src` is untouched
+│   ├── dist // Built files are now here, not in version control, ships to npm
+│   │   └── govuk // Keeps creating a namespace for Nunjucks templates
+│   ├── govuk-prototype-kit.config.json // Not in version control
+│   └── package.json // Ensures dependencies of source and build are in sync
+└── package.json // Workspaces configuration and top-level dependencies
+```
+
+
+**Date:** 2023-04-21<!-- YYYY-MM-DD -->
+
+**Status:** accepted<!-- proposed, accepted, rejected, deprecated, superseded -->
+
+## Rationale
+
+<!-- The reason for the decision and any comments or disagreements cited. -->
+
+### Regrouping source and built package files within a shared package
+
+We may need to ship with dependencies in the future. For example:
+
+- peer dependencies to a polyfill library if we shipped a version compiled with `import` statements for the polyfills (rather than having the polyfills bundled in)
+- dependencies to other libraries, if we were shipping a Babel preset or PostCSS configuration
+
+Moving the source and built files within the same package helps us ensure these dependencies are resolved consistently.
+
+We don't have any dependencies yet, but moving the files would change the shape of our package and ask people to update their import paths for Nunjucks, making it a breaking change. Taking advantage of the breaking nature of the v5 release sets us up so we don't have to worry about future dependencies.
+
+This doesn't force us to ship the source files, as well, which we've decided to exclude (at least for now). Those can be excluded via an `.npmignore` files or the `files` field in `package.json`.
+
+### Removing built package files from versioning
+
+The review app uses relative paths (`../../src/...`) to access the components of GOV.UK Frontend. While we cannot make it consume `govuk-frontend` the exact same way our users do (downloading from an `npm` registry), we can get pretty close using workspaces. Making it use `govuk-frontend` directly helps ensure that:
+- we're consuming the built files, not rebuilding from source with a process that we have to keep in sync with how the package is built
+- the actual files work as intended
+- `entries` in `package.json` are properly configured
+- dependencies get resolved as expected
+
+For that to happen, we need the package rebuilt regularly rather than only at the point of release. If we were to leave it in the repository, there'd be a risk of committing built files that are in-between the previous release and the next one. Remove it it ensures we can rebuilt at will.
+
+### Updating our release process
+
+Including the built files in version control serves as a way to check that there's no weird code sneaking in. Changes to the package get highlighted during the review of the pull request raised to commit the new build of the package and the release.
+
+We'll need to keep checking for unexplained changes to the package as we release, even if it is by manually diffing in a first time (before further automation).
+
+### Leaving `dist` in its current location
+
+`dist` doesn't need to move to enable the previous points, and with the amount of changes in v5, we'll leave it out of the equation for now.
+
+## Risks and constraints
+
+<!-- Any accepted risks associated with the decision. -->
+
+- There may be some confusion with `package` previously being somewhere not to edit the files of and now becoming where the sources are. Similarly, we'll have two `dist` folders (at root and in `package`). Naming might need updating to sort that out.
+
+## Alternatives considered
+
+<!-- Other options discussed but decided against. -->
+
+### Regrouping source and built package files within a shared package
+
+- Not regrouping the source and built package within the same package now. This would mean waiting for the next breaking change to do it. We may need to have peer dependencies sooner than it.
+
+### Removing built package files from versioning
+
+- We could also have built the package to a separate folder that's not versioned (for ex. `govuk-frontend-local-build`) to ensure we consume the built files without impacting `package`. We'd have to duplicate efforts to maintain two `package.json` files and lose testing our dependency resolution, package exports and complicate the linting and Dependabot updates.
+- Each PR could get a package deployed to the [Github npm registry](https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-npm-registry) allowing its installation by the review app down the line. This requires extra infrastructure though, and doesn't help when running locally.
+
+## Implications
+
+<!-- Impacts of this decision. -->
+
+- Users will need to update their import paths when migrating to v5, which we can document as part of the release notes/migration docs.
+- Ongoing new components (Exit This Page, Tasklist) will need to have their sources relocated before they can be merged in the new code structure. We can help with moving things and can release Exit This Page from a support branch if it needs to go before v5.
+
+## Contributors
+
+<!-- Who made this decision. -->
+- Colin Rotherham (@colinrotherham)
+- Oliver Byford (@36degrees)
+- Romaric Pascal (@romaricpascal)
+
+## Associated issues and pull requests (PRs)
+
+<!-- Add links to any related GitHub issues and / or PRs. -->
+https://github.com/alphagov/govuk-frontend/pull/3491
+https://github.com/alphagov/govuk-frontend/pull/3498
+
+## Outcomes
+
+<!-- Note any outcomes or consequences of the decision . -->
+
+Part of the discussion around this decision clarified that the review `app` does not need to take responsibility for all development processes. Performance monitoring, for example, can happily happen in its own workspaces and feed the app something to serve if necessary. Same could go for testing that the different compiled versions work appropriately it/when we start shipping more than one variant of compiled files.
+
+It was also mentioned, discussing the possibility of dependencies, that we'll need to be mindful of the implications of the dependencies we add (provenance, maintenance...).