Skip to content
This repository was archived by the owner on Jan 7, 2026. It is now read-only.

refactor: introduce monorepo-like layout #323

Merged
bajtos merged 24 commits into from
May 30, 2024
Merged

refactor: introduce monorepo-like layout #323

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
dc718ce
refactor: `lib/migrate.js` → `migrations/index.js`
bajtos May 28, 2024
a737aae
refactor: move spark-api files to a subdir
bajtos May 28, 2024
03237f0
fixup! fix tests + run per-project tests only
bajtos May 28, 2024
55f946e
Merge branch 'main' into monorepo
bajtos May 28, 2024
a202151
fixup! add mocha config
bajtos May 28, 2024
6a82285
fixup! update package lock
bajtos May 28, 2024
a88a17b
docs: update start scripts & README
bajtos May 28, 2024
3166bd7
fix: "npm run migrate"
bajtos May 28, 2024
d77af20
ci: update GHA workflow
bajtos May 28, 2024
1c4d223
fixup! build-api should test only spark-api
bajtos May 28, 2024
6e7af32
docker build using a shared Dockerfile
bajtos May 28, 2024
19c46a1
single Dockerfile shared by all pckages
bajtos May 28, 2024
554f426
remove per-project .dockerignore files
bajtos May 28, 2024
a668dae
fix GHA CI cmd for testing spark api
bajtos May 28, 2024
c7d99f5
fixup! commit .gitignore
bajtos May 29, 2024
bdbe7e9
feat: npm workspaces (#326)
bajtos May 30, 2024
fabab97
Merge branch 'main' into monorepo
bajtos May 30, 2024
dea9f21
drop `spark-` prefix from directory names
bajtos May 30, 2024
052a4b7
shared ie-contract-config and test-helpers
bajtos May 30, 2024
6371ae5
simplify Dockerfile - rework ARG SERVICE to ENV
bajtos May 30, 2024
00da105
update fly.toml to match the new Dockerfile
bajtos May 30, 2024
d964af9
ci: update the deployment command
bajtos May 30, 2024
568f597
fixup! dependabot config - remove /spark-publish
bajtos May 30, 2024
30ed273
Merge branch 'main' into monorepo
bajtos May 30, 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
5 changes: 0 additions & 5 deletions .dockerignore
View file
Open in desktop

This file was deleted.

37 changes: 11 additions & 26 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
with:
node-version: 20
- run: npm ci
- run: npm test
- run: npm run test:api

build-publish:
runs-on: ubuntu-latest
Expand All @@ -58,10 +58,9 @@ jobs:
node-version: 20
- run: npm ci
- run: npm run migrate
- run: cd spark-publish && npm ci
- run: cd spark-publish && npm test
- run: npm run test:publish

docker-build-api:
docker-build:
runs-on: ubuntu-latest
permissions:
contents: read
Expand All @@ -77,44 +76,30 @@ jobs:
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Build Docker image
- name: Build spark-api Docker image
uses: docker/build-push-action@v5
with:
context: .
build-args: "SERVICE=api"
cache-from: type=registry,ref=ghcr.io/filecoin-station/core
cache-to: type=inline

docker-build-publish:
runs-on: ubuntu-latest
permissions:
contents: read
packages: read
env:
REGISTRY: ghcr.io
steps:
- uses: actions/checkout@v4

- uses: docker/login-action@v3
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Build Docker image
- name: Build spark-publish Docker image
uses: docker/build-push-action@v5
with:
context: spark-publish
context: .
build-args: "SERVICE=publish"
cache-from: type=registry,ref=ghcr.io/filecoin-station/core
cache-to: type=inline

deploy-api:
if: github.ref == 'refs/heads/main'
needs: [build-api, build-publish, docker-build-api, docker-build-publish]
needs: [build-api, build-publish, docker-build, docker-build]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: superfly/flyctl-actions/setup-flyctl@master
- run: flyctl deploy --remote-only
- run: cd spark-api && flyctl deploy --remote-only
env:
FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
- if: failure()
Expand All @@ -138,7 +123,7 @@ jobs:
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
deploy-publish:
if: github.ref == 'refs/heads/main'
needs: [build-api, build-publish, docker-build-api, docker-build-publish]
needs: [build-api, build-publish, docker-build, docker-build]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down
15 changes: 15 additions & 0 deletions .mocharc.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const MODULES = [
'spark-api',
'spark-publish'
]

module.exports = {
spec: getSpec()
}

// Workaround for https://github.com/mochajs/mocha/issues/4100
function getSpec () {
const isTestFile = (/** @type {string} */arg) => arg.endsWith('.js')
if (process.argv.slice(2).some(isTestFile)) return []
return MODULES.map(dir => `${dir}/test/**/*.js`)
}
15 changes: 10 additions & 5 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@ WORKDIR /app
# Set production environment
ENV NODE_ENV production
ENV SENTRY_ENVIRONMENT production
ENV DOMAIN api.filspark.com
ENV REQUEST_LOGGING false

#######################################################################
# Throw-away build stage to reduce size of final image
FROM base as build

# Install packages needed to build node modules
RUN apt-get update -qq && \
apt-get install -y build-essential pkg-config python-is-python3
apt-get install -y build-essential pkg-config python-is-python3

# Install node modules
# NPM will not install any package listed in "devDependencies" when NODE_ENV is set to "production",
Expand All @@ -40,5 +38,12 @@ FROM base
# Copy built application
COPY --from=build /app /app

# Start the server by default, this can be overwritten at runtime
CMD [ "npm", "run", "start" ]
# Set to `publish` or `api`
# This argument controls what npm script is executed as the CMD.
ARG SERVICE

# ARGs are not preserved at runtime, we need to store the value
# as a default value of an ENV var
ENV SCRIPT="start:${SERVICE}"

CMD npm run ${SCRIPT}
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@ docker run -d --name spark-db \
Start the API service:

```bash
npm start
npm run start:api
Copy link
Member

@juliangruber juliangruber May 29, 2024

Choose a reason for hiding this comment

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

The only workflow downside that I see with this approach is that you can't as easily zoom in into projects any more: If I open my editor on ./spark-api/, I need to keep my CWD in . and remember to run npm run test:api. It would be nice if I could cd into that folder and just run npm test. Is this a concern for you?

What about instead we give every subrepo their own package.json and node_modules? We can still have a parent package.json on the top level, which will shell out to the sub-parent.jsons for running tests etc. I think this matches the lerna experience closer as well. But are there downsides to this approach?

Copy link
Member Author

@bajtos bajtos May 29, 2024

Choose a reason for hiding this comment

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

The only workflow downside that I see with this approach is that you can't as easily zoom in into projects any more: If I open my editor on ./spark-api/, I need to keep my CWD in . and remember to run npm run test:api. It would be nice if I could cd into that folder and just run npm test. Is this a concern for you?

Makes sense. I agree we should at least try to make this workflow easier.

I usually open my editor/IDE in the monorepo root, so this is not a concern to me.

My thoughts:

  • When you run npm in the subdirectory and there is no package.json file, npm will walk up the directory tree to the monorepo root and use the package.json file in the root.
  • However, npm test will set the working directory to the directory containing the package.json file, so it will run all tests.
  • You can run something like npx mocha test, but that's ugly.
  • You can also run npm run test:spark-api, but that's not ideal either.

We can create a small package.json file containing only the scripts:

{
  "type": "module",
  "scripts": {
    "test": "cd ..; npm run test:spark-api"
  }
}

Do you see any downsides with that?

What about instead we give every subrepo their own package.json and node_modules? We can still have a parent package.json on the top level, which will shell out to the sub-parent.jsons for running tests etc. I think this matches the lerna experience closer as well. But are there downsides to this approach?

Yes, this is the Lerna/npm workspace setup.

From what I remember, this setup makes many tasks slower:

  • npm install needs to install multiple copies of the same package when it's used by multiple components.
  • Depending on the install strategy, you can end up with all packages installed in the root node_modules folder. As a result, the code in spark-publish can import dependencies declared in spark-api only and we won't notice that.
  • We probably need to configure TypeScript with a multi-project setup. The last time I used such a setup in a large monorepo (~2021), it took seconds to minutes for tsc just to load the entire tree and decide what needs to be (re)checked.

Anyhow, I am open to giving this a try.

Copy link
Member Author

@bajtos bajtos May 29, 2024

Choose a reason for hiding this comment

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

Documentation on different install strategies for npm workspaces:

https://docs.npmjs.com/cli/v10/commands/npm-ci?v=true#configuration

Copy link
Member Author

@bajtos bajtos May 29, 2024

Choose a reason for hiding this comment

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

I opened a PR against this feature branch to try out npm workspaces: #326

Copy link
Member

@juliangruber juliangruber May 29, 2024

Choose a reason for hiding this comment

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

Do you see any downsides with that?

The downside would be that if you add a dependency from the subdirectory, it won't be added to the right place.

Yes, this is the Lerna/npm workspace setup.
From what I remember, this setup makes many tasks slower:

  • npm install needs to install multiple copies of the same package when it's used by multiple components.

To me, npm install time doesn't matter these days. I don't run it often, and the cache + my internet are fast enough. If it's a significant issue on your setup, or for your workflow, I'm happy to value this more.

  • Depending on the install strategy, you can end up with all packages installed in the root node_modules folder. As a result, the code in spark-publish can import dependencies declared in spark-api only and we won't notice that.

In which situation would all packagers be installed in the root folder? I believe that when you run cd api && npm install and there's api/package.json, then dependencies have to go to abi/node_modules/.

  • We probably need to configure TypeScript with a multi-project setup. The last time I used such a setup in a large monorepo (~2021), it took seconds to minutes for tsc just to load the entire tree and decide what needs to be (re)checked.

Ah right I remember that from our talk. I definitely don't want to introduce TS headaches here.

Copy link
Member Author

@bajtos bajtos May 29, 2024

Choose a reason for hiding this comment

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

To me, npm install time doesn't matter these days. I don't run it often, and the cache + my internet are fast enough. If it's a significant issue on your setup, or for your workflow, I'm happy to value this more.

I kind of resigned on slow npm installs, until I had a chance to experience how fast is pnpm and ever since then I am missing its speed. Switching from npm to pnpm is out of scope of this work though.

  • Depending on the install strategy, you can end up with all packages installed in the root node_modules folder. As a result, the code in spark-publish can import dependencies declared in spark-api only and we won't notice that.

In which situation would all packagers be installed in the root folder? I believe that when you run cd api && npm install and there's api/package.json, then dependencies have to go to abi/node_modules/.

npm's default configuration is to hoist dependencies. It will always install as many dependencies as possible in the root node_module folder.

install-strategy

Default: "hoisted"
Type: "hoisted", "nested", "shallow", or "linked"
Sets the strategy for installing packages in node_modules. hoisted (default): Install non-duplicated in top-level, and duplicated as necessary within directory structure. nested: (formerly --legacy-bundling) install in place, no hoisting. shallow (formerly --global-style) only install direct deps at top-level. linked: (experimental) install in node_modules/.store, link in place, unhoisted.

  • We probably need to configure TypeScript with a multi-project setup. The last time I used such a setup in a large monorepo (~2021), it took seconds to minutes for tsc just to load the entire tree and decide what needs to be (re)checked.

Ah right I remember that from our talk. I definitely don't want to introduce TS headaches here.

We also don't have TypeScript checks configured in this repository yet. I am willing to take a risk and not worry about this until I start working on adding tsc.

I think we should be fine as long as our monorepo stays small (less than 10 sub-packages, less than 100 source files).

```

Run tests and linters:

```bash
npm test
npm run test:api
```

## Deployment
Expand All @@ -105,5 +105,5 @@ Pushes to `main` will be deployed automatically.
Perform manual devops using [Fly.io](https://fly.io):

```bash
$ fly deploy
$ fly deploy spark-api
```
2 changes: 1 addition & 1 deletion bin/migrate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { migrate } from '../lib/migrate.js'
import { migrate } from '../migrations/index.js'
import pg from 'pg'

const { DATABASE_URL } = process.env
Expand Down
0 lib/migrate.js → migrations/index.js
View file
Open in desktop
File renamed without changes.
Loading