Skip to content

Commit

Permalink
Clearer documentation of how to add runtime dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
Jaifroid committed Aug 24, 2023
1 parent 5336a3d commit e22bfb0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
24 changes: 12 additions & 12 deletions ADDING_DEPENDENCIES_NODE_MODULES.md
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
# Adding runtime dependencies to the app via NPM

If you install a new RUNTIME dependency (rather than a development dependency) via NPM, and then you import the dependency into a module in the app, then you will need to take care that this dependency is available both to the unbundled and the bundled versions of the app when it is published. Although we only expect users to use the bundled version, we also expect the app to run unbundled from at least the development server https://kiwix.github.io/kiwix-js/. This is so that we can easily live-debug the app on remote systems such as BrowserStack.
If you install a new RUNTIME dependency (rather than a development dependency) via NPM, and then you import the dependency into a module in the app, then you will need to take care that this dependency is available both to the unbundled and the bundled versions of the app when it is published. Although we only expect users to use the bundled version, we also expect the app to run unbundled from at least the development server https://kiwix.github.io/kiwix-js/. This is so that we can easily live-debug the app on remote systems such as BrowserStack and pinpoint any modules causing problems.

When you import the dependency, you cannot use the "shorthand" syntax that you will often see in documentation. This is because we need the dependency to load in browser contexts and not only in contexts where NodeJS is running. So, instead of something like `import i18next from 'i18next';`, you will need to give the full path to the imported file, e.g.:

`import i18next from '../../../node_modules/i18next/dist/es/i18next.js';`

This syntax is sufficient for the bundler, rollup.js, to pick up the dependency and include it in the bundle. However, two versions of the app are published to the development server: the raw app (address https://kiwix.github.io/kiwix-js/) and the bundled app (address: https://kiwix.github.io/kiwix-js/dist/). It is the former that we need to add some support for.
This syntax is sufficient for the bundler, rollup.js, to pick up the dependency and include it in the bundle. However, two versions of the app are published to the development server: the raw app (address https://kiwix.github.io/kiwix-js/) and the bundled app (address: https://kiwix.github.io/kiwix-js/dist/). It is the former that we need to add some support for manually.

The problem is that the `node_modules` folder is usually ignored so that developers do not submit modules to the Repository. But unless we make an exception for dependencies, they won't be available to the unbundled version, so we have to take care of that.
The problem is that the `node_modules` folder is usually ignored so that developers do not submit dependencies to the Repository: instead, each developer should install dependencies using NPM. But unless we make an exception for dependencies needed in the unbundled version, they won't be available when running it in the browser, so we have to ensure they are added **only** to the version published on the dev server.

## Update the `gitignore.patch` (NOT `.gitignore`)

The app publishes to the development server after running `npm install` and bundling on the `gh-pages` branch. It uses the workflow `publish-extension.yaml`. There, you will see the following lines:
The app is published to the development server from the `gh-pages` branch. To see the process, look at the workflow `publish-extension.yaml`. It installs dependencies and builds the app. In the workflow, you will see the following lines:

```
# Apply the patch to gitignore to allow some dependencies to be included
git apply ./scripts/gitignore.patch
```

As this implies, you will need to add the dependency to the `gitignore.patch` so that it is no longer ignored when the app is published in this specific context. N.B., you must NOT directly commit the changes to `.gitignore` itself, because we do not want multiple versions of dependencies committed to the Repo and kept forever. The only branch that should have these dependencies committed is `gh-pages`, and that branch is force-pushed each time that the app is published, so only the latest versions of dependencies are committed and kept.
As this implies, you will need to add your new dependency to the `gitignore.patch` so that `.gitignore` on the `gh-pages` branch is altered, and the dependency is included in the files published to the server. N.B., you must NOT directly commit the changes to `.gitignore` itself, because we do not want multiple versions of dependencies committed to the Repo on every branch and kept forever. The only branch that should have these dependencies committed is `gh-pages`, and that branch is force-pushed each time that the app is published, so only the latest versions of dependencies are committed and kept.

To update the patch, you will need the help of software, because the format of a diff patch is a bit tricky
To update the patch, you will need the help of the git software to generate the patch, like this:

1. On your PR branch make sure your working tree is clean (that you have committed other irrelevant changes);
2. Then run the exising patch with `git apply ./scripts/gitignore.patch`. Do NOT commit these changes, but do save them;
3. Add your your dependency as well to `.gitignore`, putting it in alphabetical order into the existing block of changes, e.g.:
1. On your PR branch make sure your working tree is clean (that you have committed other irrelevant changes -- it's not necessary to have pushed the changes as well);
2. Then run the exising patch with `git apply ./scripts/gitignore.patch`. Do NOT commit the changes to `.gitignore` that this will produce, but do save those changes;
3. Now add your your dependency directly into `.gitignore`, putting it in alphabetical order into the existing block of changes, e.g.:
```
!/node_modules/i18next
/node_modules/i18next/*
Expand All @@ -34,10 +34,10 @@ To update the patch, you will need the help of software, because the format of a
!/node_modules/i18next/dist/es
!/node_modules/i18next/dist/es/*
```
This unwieldy syntax is necessary to exclude all the contents of each folder other than the file(s) you wish to include;
This unwieldy syntax is necessary to exclude all the contents of each folder other than the file(s) you wish to include. Note that the `!` at the start of some lines means "do not ignore this folder" (or file). A line without `!` means that the files listed or globbed will all be ignored;
4. Save the file (but don't commit it);
5. Run something like `git diff > mypatch.patch`;
6. Take the code in `mypatch.patch` and overwrite the code (but not the comment) in the current `./scripts/gitignore.patch`;
7. Save the chnages to `gitignore.patch`, and discard changes to `.gitignore` and `mypatch.patch`;
7. Save the chnages to `gitignore.patch`, and discard all other irrelevant changes (e.g., `.gitignore`, `mypatch.patch` and any unignored `node_module` files);
8. Test your patch works with `git apply ./scripts/gitignore.patch`;
9. If it works, discard the changes to `.gitignore` again, and you can finally push the `gitignore.patch`.
9. If it works, discard all unneeded changes escept `gitignore.patch` again, and you can finally push (just) the `gitignore.patch` to your PR.
4 changes: 4 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ To contribute code, please follow these guidelines carefully:
* _Before asking for review, thoroughly **test** (see below) both the **source code** and the **production (bundled) code** on at least Chrome/Edge and Firefox, and_
_**test that your code works as an add-on/extension** in both of these browsers. **If you have not tested your code yourself, do not expect us to test it and review it for you!**_

## Adding runtime dependencies via NPM

If your PR adds a runtime dependency with `npm install xxx` (as opposed to a development dependency with `npm install xxx --save-dev`), then you will need to ensure that your dependency files are available both in the unbundled and the bundled versions of the app, without simply committing them to `main`. You can find more details on how to do this in [ADDING_DEPENDENCIES_NODE_MODULES](./ADDING_DEPENDENCIES_NODE_MODULES.md).

## Testing

Please note that the app caches its own code so that it can run as an offline-first Progressive Web App. This can complicate development, because you may not see your changes,
Expand Down

0 comments on commit e22bfb0

Please sign in to comment.