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

Node 16 upgrade and peer dependency cleanup #250

Merged
merged 31 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6c8d611
fix: resolve most peer deps warnings in utils package
adamstankiewicz Apr 5, 2022
c0e6df1
fix: address peer dependency issues
adamstankiewicz Apr 5, 2022
3fb052c
fix: moar updates
adamstankiewicz Apr 6, 2022
ebab6bb
build: update ci.yml github action workflow
adamstankiewicz Apr 6, 2022
d73fa11
build: use node 16 to install deps
adamstankiewicz Apr 6, 2022
5a8cbf8
build: use node matrix in ci
adamstankiewicz Apr 6, 2022
95c879e
build: adds NPM workspaces to package.json
adamstankiewicz Apr 6, 2022
036ae9e
chore: upgrade @edx/frontend-platform devDeps
adamstankiewicz Apr 6, 2022
5317170
fix: update package-lock.json
adamstankiewicz Apr 6, 2022
3c68baf
fix: regenerate package-lock.json files
adamstankiewicz Apr 6, 2022
ef00ec9
build: remove workspaces from package.json
adamstankiewicz Apr 6, 2022
8763515
chore: move prop types to devDeps
adamstankiewicz Apr 6, 2022
3680827
chore: move prop-types back to deps due to eslint error
adamstankiewicz Apr 6, 2022
e8d7c89
build: use npm i vs npm ci
adamstankiewicz Apr 6, 2022
8a8dcf9
fix: push up local changes
adamstankiewicz Apr 13, 2022
851a12f
fix: add extendJestConfig helper function
adamstankiewicz Apr 13, 2022
80c581e
fix: make things work
adamstankiewicz Apr 13, 2022
77941a0
build: only run CI in Node 16
adamstankiewicz Apr 13, 2022
8b9be68
build: run CI workflow on pull_request actions
adamstankiewicz Apr 13, 2022
9e05565
build: only install npm packages at root with workspaces
adamstankiewicz Apr 13, 2022
3710f45
build: only install npm packages at root with workspaces pt2
adamstankiewicz Apr 13, 2022
b20474a
build: update how branch name is retrieved in version dry run
adamstankiewicz Apr 13, 2022
7bd6dce
build: only install npm packages at root with workspaces pt2 again
adamstankiewicz Apr 13, 2022
8200d01
build: try to fix detached head issue
adamstankiewicz Apr 13, 2022
3d5a36e
build: try to fix detached head issue pt2
adamstankiewicz Apr 13, 2022
6198016
build: remove eslint-import-resolver-alias
adamstankiewicz Apr 13, 2022
3c517b3
build: run workspaces command using npm
adamstankiewicz Apr 14, 2022
ea3cfb3
build: update npm scripts to use npm workspaces and update readme
adamstankiewicz Apr 14, 2022
53f4fa1
feat: drop support for Node 12 and add support for Node 16
adamstankiewicz Apr 14, 2022
08b7563
build: attempt to fetch tags in checkout workflow step
adamstankiewicz Apr 14, 2022
465f67f
build: move back to pulling tags as separate step
adamstankiewicz Apr 14, 2022
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
4 changes: 2 additions & 2 deletions .github/workflows/bump-all-major.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Setup Nodejs
uses: actions/setup-node@v1
with:
node-version: 12
node-version: 16
# lerna expects to be authenticated for publishing to NPM. This step will fail CI if NPM is not authenticated
- name: Check NPM authentication
run: |
Expand All @@ -35,7 +35,7 @@ jobs:
run: npm run test
- name: Coverage Report
uses: codecov/codecov-action@v1
- name:
- name: Force publish major versions
run: lerna version major --force-publish --yes
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
76 changes: 37 additions & 39 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,46 +1,44 @@
name: CI

on: [push]
on: [pull_request]

jobs:
build:
tests:
name: Build
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
with:
# pulls all commits (needed for lerna / semantic release to correctly version)
fetch-depth: 0
# pulls all tags (needed for lerna / semantic release to correctly version)
- run: git fetch --depth=1 origin +refs/tags/*:refs/tags/*
- name: Setup Nodejs
uses: actions/setup-node@v1
with:
node-version: 12
# lerna expects to be authenticated for publishing to NPM. This step will fail CI if NPM is not
# authenticated, even though this build does _not_ attempt to publish, as an extra check before merge
# that Lerna is set to publish.
- name: Check NPM authentication
run: |
echo "//registry.npmjs.org/:_authToken=${{ secrets.SEMANTIC_RELEASE_NPM_TOKEN }}" >> .npmrc
npm whoami
- name: Install and Setup Dependencies
run: |
npm install
lerna bootstrap --include-dependencies --include-dependents --since origin/master
# build must come before running linting and tests for the `dist` directory to exist.
- name: Build
run: lerna run build --include-dependencies --include-dependents --since origin/master --parallel
- name: Lint
run: lerna run lint --include-dependencies --include-dependents --since origin/master --parallel
- name: Test
run: lerna run test --include-dependencies --include-dependents --since origin/master --parallel
- name: Coverage Report
uses: codecov/codecov-action@v1
- name: Extract branch name
shell: bash
run: echo "##[set-output name=branch;]$(echo ${GITHUB_REF#refs/heads/})"
id: extract_branch
- name: Preview Updated Versions (dry run)
run: lerna version --allow-branch ${{ steps.extract_branch.outputs.branch }} --no-git-tag-version --no-changelog --no-push --yes
- name: Checkout
uses: actions/checkout@v2
with:
# pulls all commits (needed for lerna / semantic release to correctly version)
fetch-depth: 0
# pulls all tags (needed for lerna / semantic release to correctly version)
- name: Pull all tags
run: git fetch --depth=1 origin +refs/tags/*:refs/tags/*
- name: Setup Nodejs
uses: actions/setup-node@v3
with:
node-version: 16
Copy link
Member Author

@adamstankiewicz adamstankiewicz Apr 13, 2022

Choose a reason for hiding this comment

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

This repository is now relying on NPM workspaces. As such, it is not compatible with Node 12 or Node 14, as NPM workspaces require npm v7+, so we will only run against Node 16.

# lerna expects to be authenticated for publishing to NPM. This step will fail CI if NPM is not
# authenticated, even though this build does _not_ attempt to publish, as an extra check before merge
# that Lerna is set to publish.
- name: Check NPM authentication
run: |
echo "//registry.npmjs.org/:_authToken=${{ secrets.SEMANTIC_RELEASE_NPM_TOKEN }}" >> .npmrc
npm whoami
- name: Install and Setup Dependencies
run: npm run setup
# build must come before running linting and tests for the `dist` directory to exist.
- name: Build
run: npm run build
- name: Lint
run: npm run lint
- name: Test
run: npm run test
- name: Coverage Report
uses: codecov/codecov-action@v2
- name: Preview Updated Versions (dry run)
# switch from detatched head to a real branch so we can pass it to `--allow-branch`
run: |
git switch -c "actual-branch-not-detached-head"
lerna version --allow-branch actual-branch-not-detached-head --no-git-tag-version --no-changelog --no-push --yes
Comment on lines +43 to +44
Copy link
Member Author

Choose a reason for hiding this comment

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

By changing the on from "push" to "pull_request" ensures this workflow is only run when necessary, but in doing so, the checkout is in a detached HEAD state, so we need to force switch to an attached branch for the version dry-run command to work.

2 changes: 1 addition & 1 deletion .github/workflows/publish-from-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Setup Nodejs
uses: actions/setup-node@v1
with:
node-version: 12
node-version: 16
# lerna expects to be authenticated for publishing to NPM. This step will fail CI if NPM is not authenticated
- name: Check NPM authentication
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
- name: Setup Nodejs
uses: actions/setup-node@v1
with:
node-version: 12
node-version: 16
# lerna expects to be authenticated for publishing to NPM. This step will fail CI if NPM is not authenticated
- name: Check NPM authentication
run: |
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
12
16
19 changes: 7 additions & 12 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,8 @@ To get started with ``frontend-enterprise`` local development, clone the repo an
::

npm run setup
npm run dev

The above commands will install package dependencies, symlinking local packages rather than installing from NPM directly. However, since (most of) the packages in this monorepo contain a build step for Babel transpilation, it necessary to watch changes in the ``src`` directories of each package to re-run the relevant build script(s) as needed (``npm run dev``).

Each package is configured to only publish the contents of the generated ``dist`` directory to NPM; this behavior gets replicated in the local symlinking of packages as well, which is why it is necessary to watch the source files for changes and re-build the ``dist`` directory when they do change. This ensures the symlinked packages will always contain the correct packages. It is recommended to run ``npm run dev`` in a separate terminal tab or window so you may run other commands as needed while source files are still being watched/transpiled.
The above command will install package dependencies using NPM workspaces, hoisting all packages to `node_modules` at the root of the repository for performance reasons (e.g., there will only be one copy of React). By using NPM workspaces, `npm install` knows that when importing a package that is part of this monorepo (e.g., `@edx/frontend-enterprise-utils`), it should look at the local package folder and creates symlinks accordingly.

Other useful commands for linting and testing may include:

Expand All @@ -32,20 +29,18 @@ Other useful commands for linting and testing may include:
npm run lint
npm run test

The above NPM scripts are running Lerna commands behind-the-scenes. By default, it will run the associated NPM command in each package in the monorepo. However, Lerna provides a mechanism to only run tests for a specific package(s), for example:
The above NPM scripts are run via NPM workspaces behind-the-scenes. By default, it will run the associated NPM command in each package in the monorepo. However, NPM workspaces does provide a mechanism to only run tests for a specific package, for example:

::

lerna run test --scope=@edx/frontend-enterprise-catalog-search
npm run test -w @edx/frontend-enterprise-catalog-search

To clean your local monorepo of any installed ``node_modules`` and symlinked packages to start fresh, you may run:

::

npm run clean

See https://github.com/lerna/lerna for full documentation of Lerna commands.

Installing local monorepo package(s) from an edX micro-frontend
-----

Expand Down Expand Up @@ -83,9 +78,7 @@ Each package in the monorepo contains its own package.json file and unique set o

To get around this issue of common/shared dependencies, we can rely on how NPM finds installed packages. If a package does not exist in ``node_modules`` for an individual package, NPM will look in ``node_modules`` further up the directory tree until it finds the package, or gets to the root of the repository.

By installing these common dependencies at the root package.json file, they will be accessible to any package in the monorepo to ensure there is only one copy of them used throughout. These dependencies are still noted in each individual package.json file as a peer dependency but not as a dev dependency since they are already installed in ``node_modules`` at the root of the repository.

As such, we should pay extra attention to managing dependencies in each packages, making informed decisions about whether a dependency should be included in an individual package's package.json file or the package.json file at the root of the repository.
NPM workspaces helps with this by hoisting installed packages to the root `node_modules` folder where they will be accessible to any package in the monorepo to ensure there is only one copy used throughout. These dependencies are still noted in each individual package.json file as both a peer dependency and a dev dependency.

Writing a commit
-----
Expand All @@ -96,6 +89,8 @@ There is a precommit plugin (commitlint) which requires commit messages formatte

where type must be one of ``[build, ci, docs, feat, fix, perf, refactor, revert, style, test]``

Note: only `fix`, `feat`, and `perf` will trigger a new NPM release, as this is the default behavior for semantic-release.

Versioning and releases
*****

Expand All @@ -108,7 +103,7 @@ To publish the packages that had their versions incremented, you must manually t
Preview changed packages in CI with Github Actions
-----

As a convenience, the ``lerna changed`` command is run for each push to determine which packages in the monorepo will be published should a PR get merged.
As a convenience, a dry run of the ``lerna version`` command is run for each push to determine which packages in the monorepo will be published should a PR get merged.

.. |Build Status| image:: https://github.com/edx/frontend-enterprise/actions/workflows/release.yml/badge.svg
:target: https://github.com/edx/frontend-enterprise/actions
Expand Down
13 changes: 13 additions & 0 deletions common/extendESLintConfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const path = require('path');

const extendESLintConfig = (config) => {
const importNoUnresolved = config.rules['import/no-unresolved'];
if (importNoUnresolved ) {
const originalIgnore = importNoUnresolved[1].ignore;
if (!originalIgnore.includes('@edx/frontend-enterprise-*')) {
importNoUnresolved[1].ignore = [...originalIgnore, '@edx/frontend-enterprise-*'];
}
}
};

module.exports = extendESLintConfig;
10 changes: 10 additions & 0 deletions common/extendJestConfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const extendJestConfig = (config) => {
const modulePathsToIgnore = ['<rootDir>/dist'];
config.modulePathIgnorePatterns = config.modulePathIgnorePatterns ? [...config.modulePathIgnorePatterns, ...modulePathsToIgnore] : modulePathsToIgnore;
config.moduleNameMapper = {
...config.moduleNameMapper,
'@edx/frontend-enterprise-(.*)': '<rootDir>/../$1/src',
};
};

module.exports = extendJestConfig;
Loading