[UIE-94] Multi-package MVP #3992

nawatts · 2023-07-06T17:06:57Z

https://broadworkbench.atlassian.net/browse/UIE-94

This sets up the infrastructure for and begins splitting up Terra UI into multiple packages. As a proof of concept / MVP, this extracts the Interactive component into @terra-ui-packages/components and the contents of src/libs/type-utils into @terra-ui-packages/core-utils.

There are several pieces involved in letting us import { Interactive } from '@terra-ui-packages/components' in Terra UI application code...

Yarn workspaces are used to have imports from @terra-ui-packages/components in Terra UI resolve to that package's directory within the packages directory (./packages/components).
https://yarnpkg.com/features/workspaces

exports, module, and main configuration in each package's package.json file control how imports of the package are resolved to a file. Each package is configured to have two versions: an ES Module version in lib/es/index.js and a CommonJS version in lib/cjs/index.js. We want the ES Module version to use with a bundler (Vite) and the CommonJS version to use with Jest (since Jest doesn't yet support modules). So with the package.json configuration, import { ... } from '@terra-ui-packages/components' resolves to ./packages/components/lib/es/index.js and require('@terra-ui-packages/components') resolves to ./packages/components/lib/cjs/index.js.
https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

The files in the lib directory are outputs from a build. Each package has a build script configured in package.json. That build script runs tsc twice: once to produce the ES Module output and once to produce the CommonJS output. Those outputs are configured in two TS project files: tsconfig.json has configuration for ES Modules and contains any package specific TS settings that are needed. tsconfig.cjs.json extends tsconfig.json and overrides the module and outDir settings. Each package's tsconfig.json extends from tsconfig.base.json at the workspace root so that all packages can share common base compiler settings.

There's also a tsconfig.json at the workspace root. Currently, it has two functions... It serves as the TS project for the Terra UI application itself (ideally, the application would be a separate package instead of mingled with the workspace root). It also contains references to the TS projects for each package. This allows building all packages from the workspace root with tsc --build.
https://www.typescriptlang.org/docs/handbook/project-references.html

To have changes to packages code automatically built and reloaded in the browser, as we're used to with application code, the start script now runs two things: the Vite dev server and tsc --build --watch.

Changes to development workflow:

yarn run build does not change
yarn start is used as before, though now it runs an additional process (tsc --build --watch)
yarn test ...
- No longer runs all tests in the repo. It now runs only tests for the Terra UI application.
- Terra UI relies on the build output of packages, so some care must be taken to ensure that package builds are up to date before running Terra UI tests to ensure that the current packages code is being tested. Also, a build has to be run (yarn run build or yarn start) before running tests for the first time.

To run a script for a specific package, use yarn workspace. For example, to run tests for core-utils, use yarn workspace '@terra-ui-packages/core-utils' test.
https://yarnpkg.com/cli/workspace

To run a script for all packages, use yarn workspaces foreach. For example, to run unit tests for all packages and Terra UI, use yarn workspaces foreach --exclude terra-integration-tests run test.
https://yarnpkg.com/cli/workspaces/foreach

nawatts · 2023-07-10T20:47:16Z

.circleci/config.yml

+      - run: yarn run build
      - run: yarn eslint --max-warnings=0 .
-      - run: yarn test --coverage --maxWorkers=2
+      - run: yarn workspaces foreach --exclude terra-integration-tests run test --coverage --maxWorkers=2 --passWithNoTests
      - run: yarn check-types
-      - run: DISABLE_ESLINT_PLUGIN=true yarn build # already linted above


ESLint and tests require the packages to be built first so that imports from the packages can be resolved.

nawatts · 2023-07-10T20:47:25Z

.eslintignore

@@ -13,4 +13,4 @@ node_modules
 .vscode
 coverage
 vite.config.js
-jest/*
+packages/*/lib


Ignore build outputs.

nawatts · 2023-07-10T20:48:47Z

.gitignore

@@ -5,9 +5,12 @@ node_modules

 # testing
 /coverage
+packages/*/coverage


A coverage report is generated for each package.

nawatts · 2023-07-10T20:48:54Z

.gitignore


 # production
 /build
+packages/*/lib
+tsconfig.tsbuildinfo


Ignore build outputs.

nawatts · 2023-07-10T20:50:24Z

.yarnrc.yml

+  - path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
+    spec: "@yarnpkg/plugin-workspace-tools"


This plugin provides yarn workspaces foreach ....
https://yarnpkg.com/cli/workspaces/foreach

nawatts · 2023-07-10T20:53:18Z

Dockerfile

@@ -1,7 +1,9 @@
 FROM node:18.10

 COPY . /terra-ui/
-RUN cd /terra-ui && yarn build
+RUN cd terra-ui \
+  && PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true yarn install --immutable \


Run yarn install to build "unplugged" packages (such as ESBuild).

Without PUPPETEER_SKIP_CHROMIUM_DOWNLOAD =true, Puppeteer's build failed because "The chromium binary is not available for arm64.".

yarn install is currently being run from the yarn build script, but this PR removes it from that step.

nawatts · 2023-07-10T20:54:25Z

package.json

+    "@terra-ui-packages/components": "*",
+    "@terra-ui-packages/core-utils": "*",


The Terra UI app depends on the latest code in each package. Yarn resolves imports from these packages to the files in the packages directory.

nawatts · 2023-07-10T20:55:19Z

package.json

@@ -64,20 +66,22 @@
  },
  "scripts": {
    "analyze": "yarn build && source-map-explorer 'build/static/js/*.js' --gzip",
-    "build": "yarn install && env $(yarn setenv) vite build && rm build/config.json",
+    "build": "tsc --build && env $(yarn setenv) vite build && rm build/config.json",


tsc --build builds each package and checks types. The packages must be built so that Vite can resolve imports from them.

nawatts · 2023-07-10T20:56:25Z

package.json

    "serve": "vite preview",
-    "start": "env $(yarn setenv) vite",
+    "start": "concurrently \"tsc --build --watch\" \"env $(yarn setenv) vite\"",


tsc --build --watch watches for changes to packages code and rebuilds them. vite watches for changes to Terra UI code and packages build output and rebuilds Terra UI.

nawatts · 2023-07-10T20:57:08Z

package.json

-    "check-types": "check-dts types/**/*.ts",
+    "test": "jest",
+    "check-dts": "check-dts types/**/*.ts",
+    "check-types": "yarn workspaces foreach run check-dts",


@terra-ui-packages/core-utils includes type helpers and also has a check-dts script to check those.

nawatts · 2023-07-10T21:02:28Z

packages/components/package.json

+  "version": "0.0.1",
+  "private": true,
+  "scripts": {
+    "build": "tsc && tsc -p tsconfig.cjs.json",


We build two different versions of each package (ES Modules and CommonJS).

Should this have a comment inline describing why we need two versions?

Off the top of my head, I'm not sure if NPM / Yarn can tolerate comments in package.json files. This could probably be clarified some by having the first tsc explicitly state the config file (tsconfig.json) vs relying on the default. The tsconfig files have comments about what they are for.

nawatts · 2023-07-10T21:03:27Z

packages/components/package.json

+  "type": "module",
+  "module": "./lib/es/index.js",
+  "main": "./lib/cjs/index.js",
+  "exports": {
+    ".": {
+      "import": "./lib/es/index.js",
+      "require": "./lib/cjs/index.js"
+    }
+  },


Configure exports for using this package via import (ES Modules) or require (CommonJS). The package API is the same either way, but they resolve to different files.

nawatts · 2023-07-10T21:03:44Z

packages/components/package.json

+  "files": [
+    "lib/cjs/**",
+    "lib/es/**"
+  ],


Only include build outputs in the published package.

nawatts · 2023-07-10T21:09:34Z

packages/core-utils/src/timer-utils.ts

@@ -0,0 +1,3 @@
+export const delay = (ms: number): Promise<void> => {


delay was pulled into this PR because a type-utils test depends on it.

nawatts · 2023-07-10T21:10:40Z

packages/core-utils/src/type-utils/LoadedState.ts

-type LoadedState<S, E = Error> = NoneState | LoadingState<S> | ReadyState<S> | ErrorState<S, E>;
-
-export default LoadedState;
+export type LoadedState<S, E = Error> = NoneState | LoadingState<S> | ReadyState<S> | ErrorState<S, E>;


Since all imports will be from the package (import { LoadedState } from '@terra-ui-packages/core-utils';), all exports must be named.

nawatts · 2023-07-10T21:11:47Z

packages/core-utils/tsconfig.json

+    "rootDir": "src"
+  },
+  "include": ["src"],
+  "exclude": ["src/**/*.types.ts", "src/**/*.errors.ts"]


Exclude the files used by check-dts. lodash-fp-types.errors.ts intentionally includes invalid code and if it were included, the project would not build.

nawatts · 2023-07-10T21:21:55Z

sonar-project.properties

-sonar.javascript.lcov.reportPaths=coverage/lcov.info
+sonar.javascript.lcov.reportPaths=coverage/lcov.info,packages/*/coverage/lcov.info


Each package now has its own coverage report. Upload them all to SonarCloud.

nawatts · 2023-07-10T21:22:35Z

sonar-project.properties


-# Only scan code under src.
-sonar.sources=src
+sonar.sources=packages,src


The packages directory now also contains source code.

nawatts · 2023-07-10T21:23:11Z

tsconfig.base.json

Base TypeScript configuration for all packages and Terra UI itself.

nawatts · 2023-08-02T21:43:16Z

Rebased to resolve merge conflict.

nawatts · 2023-08-02T22:01:46Z

.hooks/plugin-warning-logger.js

@@ -16,7 +16,9 @@ module.exports = {
        }
      },
      wrapScriptExecution(executor, project, locator, scriptName) {


This is what shows the

╔══════════════════════════════════════════════════════════════════════════════╗ ║ Be sure to copy a config/xxx.json to build/config.json if you're planning to ║ ║ deploy this build. ║ ╚══════════════════════════════════════════════════════════════════════════════╝

notice after running a build. We only want to show that on app builds, not package builds.

There's definitely easier ways to show this notice than a Yarn hook, but I'll leave it be here.

The copying over happens as part of our build process? I guess I'm wondering why we have this warning at all.

Yes, it's done in CI.

terra-ui/.circleci/config.yml

Line 117 in ecc6bf1

- run: cp config/<< parameters.env >>.json build/config.json

https://github.com/DataBiosphere/saturn-ui-prod-deploy/blob/44b4565a4dc118482be63e12d6497ab62271fcb0/.circleci/config.yml#L76

Yeah, we could probably get rid of this altogether. Though it might be worth keeping just for the rare case of manual deploys.

That is exactly why it is there. If I had the time, I'd instead move this warning into an error message that shows up when the UI attempts to pull that file and finds it absent. This warning was added specifically because the mechanism for resolving that error isn't particularly obvious.

cahrens · 2023-08-03T13:12:14Z

packages/components/package.json

+  ],
+  "dependencies": {
+    "lodash": "^4.17.21",
+    "react-hyperscript-helpers": "^2.0.0"


I don't see where it uses react-hyperscirpt-helpers.

You're right, it doesn't. I think I intended to use it for Interactive, but then ran into a stumbling block: #3992 (comment).

I can remove it, but any future components added to this package will likely use it.

I would tend to not including it until it's actually used, but given that I was OK with leaving the test-utils dependency in even though it isn't used yet I guess it's not a big deal.

Removed in 83b8c59.

sonarcloud · 2023-08-03T13:31:42Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

26.7% Coverage
4.8% Duplication

The version of Java (11.0.11) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

nawatts requested review from cahrens and msilva-broad July 6, 2023 17:07

broadbot temporarily deployed to pr-10 July 6, 2023 17:13 Inactive

broadbot temporarily deployed to pr-10 July 6, 2023 17:28 Inactive

broadbot temporarily deployed to pr-10 July 6, 2023 17:59 Inactive

broadbot temporarily deployed to pr-10 July 6, 2023 18:08 Inactive

nawatts force-pushed the uie-94-multi-package branch from f3967fc to a3f551b Compare July 10, 2023 19:22

broadbot temporarily deployed to pr-10 July 10, 2023 19:32 Inactive

broadbot temporarily deployed to pr-10 July 10, 2023 20:28 Inactive

broadbot temporarily deployed to pr-10 July 10, 2023 20:46 Inactive

nawatts commented Jul 10, 2023

View reviewed changes

.gitignore

# production

/build

packages/*/lib

tsconfig.tsbuildinfo

Copy link

Contributor Author

nawatts Jul 10, 2023

Choose a reason for hiding this comment

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

Ignore build outputs.

nawatts commented Jul 10, 2023

View reviewed changes

broadbot temporarily deployed to pr-10 July 10, 2023 20:57 Inactive

nawatts commented Jul 10, 2023

View reviewed changes

tsconfig.base.json

Copy link

Contributor Author

nawatts Jul 10, 2023

Choose a reason for hiding this comment

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

Base TypeScript configuration for all packages and Terra UI itself.

nawatts added 8 commits August 2, 2023 17:39

Remove type parameters from types based on InteractiveProps

94678b8

Fix type of Interactive's children prop

7345440

Use Interactive from components package

5b26c74

Replace type-utils with core-utils package

ddc63f8

Fix Docker build

fbca524

Remove comment

a360a32

Remove baseUrl setting

873327a

Rename prop

77ef903

nawatts force-pushed the uie-94-multi-package branch from 3a7f125 to 77ef903 Compare August 2, 2023 21:43

nawatts requested a review from cahrens August 2, 2023 21:43

broadbot temporarily deployed to pr-10 August 2, 2023 21:50 Inactive

Add comments to tsconfig files

5518726

nawatts commented Aug 2, 2023

View reviewed changes

Only show config notice for app build

34f9b83

nawatts force-pushed the uie-94-multi-package branch from b76a958 to 34f9b83 Compare August 2, 2023 22:06

broadbot temporarily deployed to pr-10 August 2, 2023 22:15 Inactive

Update docker ignore file

fee309b

broadbot temporarily deployed to pr-10 August 2, 2023 23:00 Inactive

cahrens reviewed Aug 3, 2023

View reviewed changes

cahrens approved these changes Aug 3, 2023

View reviewed changes

cahrens removed the request for review from marctalbott August 3, 2023 13:22

Remove react-hyperscript-helpers from components dependencies

83b8c59

broadbot temporarily deployed to pr-10 August 3, 2023 13:32 Inactive

nawatts added this pull request to the merge queue Aug 3, 2023

Merged via the queue into dev with commit 14a0fa6 Aug 3, 2023
5 checks passed

nawatts deleted the uie-94-multi-package branch August 3, 2023 17:27

dmohs mentioned this pull request Aug 9, 2023

[risk=no][no ticket] Add and use Interactive package from Terra UI all-of-us/workbench#7868

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UIE-94] Multi-package MVP #3992

[UIE-94] Multi-package MVP #3992

nawatts commented Jul 6, 2023 •

edited

Loading

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Aug 2, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

dmohs Aug 4, 2023

nawatts Aug 4, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts Jul 10, 2023 •

edited

Loading

nawatts Jul 10, 2023

nawatts Jul 10, 2023 •

edited

Loading

nawatts Jul 10, 2023

nawatts Jul 10, 2023

nawatts commented Aug 2, 2023

nawatts Aug 2, 2023 •

edited

Loading

cahrens Aug 3, 2023

nawatts Aug 3, 2023 •

edited

Loading

dmohs Aug 4, 2023

cahrens Aug 3, 2023

nawatts Aug 3, 2023 •

edited

Loading

cahrens Aug 3, 2023

nawatts Aug 3, 2023

sonarcloud bot commented Aug 3, 2023

		- path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
		spec: "@yarnpkg/plugin-workspace-tools"

		"@terra-ui-packages/components": "*",
		"@terra-ui-packages/core-utils": "*",

		@@ -0,0 +1,3 @@
		export const delay = (ms: number): Promise<void> => {

		sonar.javascript.lcov.reportPaths=coverage/lcov.info
		sonar.javascript.lcov.reportPaths=coverage/lcov.info,packages/*/coverage/lcov.info

[UIE-94] Multi-package MVP #3992

[UIE-94] Multi-package MVP #3992

Conversation

nawatts commented Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nawatts Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nawatts Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nawatts commented Aug 2, 2023

nawatts Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nawatts Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nawatts Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 3, 2023

nawatts commented Jul 6, 2023 •

edited

Loading

nawatts Jul 10, 2023 •

edited

Loading

nawatts Jul 10, 2023 •

edited

Loading

nawatts Aug 2, 2023 •

edited

Loading

nawatts Aug 3, 2023 •

edited

Loading

nawatts Aug 3, 2023 •

edited

Loading