Yarn v3 upgrade fixes #4029

IgorNadj · 2022-07-14T05:25:18Z

Followup fixes to yarn v1 -> v3 upgrade #3994

Cleans up __tests__ dirs for consistency
Upgrade nohoist for meditrak-app
Move default babel config flags to base package:build:js script
Each package cleans their dist before building
Use standard test runners and builders for consistency

testUtilities are used by other packages, so should be compiled into dist, __tests__ should not be in dist

Previous way of defining ignored paths in build flags no longer works in yarn 3 way. Moved ignore paths to .babelrc files

Make it consistent with all other ts packages

biaoli0

LGTM! Seems like there is a testing error in web-config-server from codeship, wanna have a quick fix?

IgorNadj · 2022-07-15T04:55:34Z

package.json

@@ -34,7 +34,7 @@
    "test-all": "node ./scripts/node/testAll --silent",
    "validate": "./scripts/bash/validate.sh",
    "package:build:ts": "cd $INIT_CWD && tsc -p tsconfig-build.json",
-    "package:build:js": "cd $INIT_CWD && babel src --out-dir dist --source-maps --config-file \"../../babel.config.json\"",
+    "package:build:js": "cd $INIT_CWD && babel src --out-dir dist --source-maps --config-file \"../../babel.config.json\" --copy-files --no-copy-ignored --ignore \"**/__tests__\",\"**/__mocks__\"",


These flags are the default now

IgorNadj · 2022-07-15T04:56:08Z

packages/admin-panel-server/package.json

@@ -15,7 +15,7 @@
  "types": "dist/index.d.ts",
  "private": true,
  "scripts": {
-    "build": "npm run --prefix ../../ package:build:ts",
+    "build": "rm -rf dist && npm run --prefix ../../ package:build:ts",


Every package cleans their dist before building to avoid issues with stale files

IgorNadj · 2022-07-15T04:58:21Z

packages/kobo-api/package.json

    "build:types": "npm run --prefix ../../ package:build:types",
    "build-dev": "npm run build",
-    "test": "nyc --reporter=lcov mocha",
-    "test-coverage": "cross-env NODE_ENV=test nyc mocha"
+    "test": "yarn package:test",


Use standard test runner (jest) instead of mocha

IgorNadj · 2022-07-15T04:59:31Z

packages/central-server/package.json

@@ -13,7 +13,7 @@
  "types": "dist/index.d.ts",
  "scripts": {
    "build": "rm -rf dist && run-p -c \"build:* {@}\" --",
-    "build:src": "yarn package:build:js --ignore \"**src/database/migrations\",\"src/tests/**\" --copy-files --no-copy-ignored",
+    "build:src": "yarn package:build:js",


Uses standard build. The ignore flags are different... so central server outputs test files and migrations to dist now, but it shouldn't be too much of an issue since central-server is never imported by another package.

Can we ignore them to make the bundle file smaller?

I came back to this with a fresh head and moved these flags to the .babelrc.js file. This way we can use the standard build script as well as customise the build babel config for central-server.

We are now back to 9MB builds, 3MB smaller (same as dev), and as fast as dev.

IgorNadj · 2022-07-15T05:01:46Z

packages/meditrak-app/.yarnrc.yml

@@ -0,0 +1 @@
+nmHoistingLimits: workspaces


Fixes an issue Andrew found with building meditrak-app locally.

yarn v3 doesn't support nohoist, but instead we have to say don't hoist anything.

IgorNadj · 2022-07-15T05:02:29Z

packages/server-boilerplate/src/index.ts

@@ -20,4 +20,4 @@ export {
 } from './orchestrator';
 export * from './types';
 export * from './models';
-export { TestableServer } from './__tests__/utilities';
+export { TestableServer } from './testUtilities';


Moved testUtilities out of __tests__ because __tests__ doesn't get ouput into dist, and other packages need these testUtilities.

IgorNadj · 2022-07-15T05:02:58Z

packages/tsutils/package.json

@@ -12,7 +12,7 @@
  "main": "dist/index.js",
  "types": "dist/index.d.ts",
  "scripts": {
-    "build": "rm -rf dist && tsc",
+    "build": "rm -rf dist && npm run --prefix ../../ package:build:ts",


Use standard ts build script

IgorNadj · 2022-07-15T05:04:14Z

packages/ui-components/src/__tests__/components/Chart/parseChartConfig.test.js

@@ -4,8 +4,11 @@
 *
 */

-import { parseChartConfig } from '../parseChartConfig';
-import { CHART_COLOR_PALETTE, EXPANDED_CHART_COLOR_PALETTE } from '../constants';
+import { parseChartConfig } from '../../../components/Chart/parseChartConfig';


Update ui-components folder structure to put all tests under __tests__ instead of alongside each component for consistency with all other packages.

IgorNadj · 2022-07-15T05:04:37Z

packages/weather-api/package.json

    "build:types": "npm run --prefix ../../ package:build:types",
    "build-dev": "npm run build",
-    "test": "nyc --reporter=lcov mocha",
-    "test-coverage": "cross-env NODE_ENV=test nyc mocha"
+    "test": "yarn package:test",


Use standard test runner

IgorNadj · 2022-07-15T06:12:09Z

@billli0 Went too far in my original changes, had to roll back some packages. I was hoping we could get our old packages like web-config-server to use the normal build and test configs, but they have special needs so I rolled them back to use their old custom build and test scripts. Ready for re-review.

biaoli0

New changes look good, thanks for the fix. Definitely it would be nice to have web-config-server updated to be more standardised as other packages.
You mentioned the web-config-server have its own needs, what would that be? Will it be a big one for us the refactor?

IgorNadj · 2022-07-18T04:47:51Z

New changes look good, thanks for the fix. Definitely it would be nice to have web-config-server updated to be more standardised as other packages. You mentioned the web-config-server have its own needs, what would that be? Will it be a big one for us the refactor?

There are still a few packages that use mocha. To get these to use the standard test runner (jest) we will need to go through all tests and migrate them, some of the syntax is different, like expect().to.be instead of expect().toBe or something like that. Not too big but I thought it is probably out of scope for this PR.

Go back to ignoring test,migration files for building

biaoli0 · 2022-07-18T05:17:19Z

There are still a few packages that use mocha. To get these to use the standard test runner (jest) we will need to go through all tests and migrate them, some of the syntax is different, like expect().to.be instead of expect().toBe or something like that. Not too big but I thought it is probably out of scope for this PR.

Yeah I'm just a little bit curious. It is definitely out of scope.
I was working on web-config-server unit test and I'm not really satisfied with mocha.. I have some interest to migrate that, will see how that goes.

IgorNadj added 5 commits July 14, 2022 13:07

Cleanup: move docs out of src

078e269

Cleanup: move testUtilities out of __tests__

b987c7f

testUtilities are used by other packages, so should be compiled into dist, __tests__ should not be in dist

Remove tests from dist/

6978a74

Previous way of defining ignored paths in build flags no longer works in yarn 3 way. Moved ignore paths to .babelrc files

Clean dist/ before build in all packages

496c468

Cleanup tsutils tsconfig

68720a9

Make it consistent with all other ts packages

biaoli0 suggested changes Jul 15, 2022

View reviewed changes

IgorNadj added 5 commits July 15, 2022 12:59

Move all packages' tests to __tests__, common config

97e0a17

Restore test config central-server

74197af

Restore web-config-server tests dir

30de1da

Migrate nohoist to yarn 3 format

eb157e4

Weather api use normal test

a1f85b0

IgorNadj changed the title ~~Remove tests from dist~~ Yarn v3 upgrade fixes Jul 15, 2022

IgorNadj commented Jul 15, 2022

View reviewed changes

Refactor: move web-frontend tests back to tests/

55d1537

IgorNadj requested a review from biaoli0 July 15, 2022 06:12

biaoli0 approved these changes Jul 18, 2022

View reviewed changes

Remove build flags, these are defaults now

72ebb27

IgorNadj added 2 commits July 18, 2022 15:01

Fix central-server build size

537a7ef

Go back to ignoring test,migration files for building

Add missing nyc devDeps

ccb2b7a

IgorNadj merged commit ad9f0a7 into dev Jul 18, 2022

IgorNadj deleted the remove-tests-from-dist branch July 18, 2022 05:59

IgorNadj mentioned this pull request Jul 18, 2022

Release 2022-30 #4028

Closed

biaoli0 mentioned this pull request Jul 25, 2022

Release 2022-30 #4041

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Yarn v3 upgrade fixes #4029

Yarn v3 upgrade fixes #4029

IgorNadj commented Jul 14, 2022 •

edited

Loading

biaoli0 left a comment

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

biaoli0 Jul 18, 2022

IgorNadj Jul 18, 2022

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

IgorNadj Jul 15, 2022

IgorNadj commented Jul 15, 2022 •

edited

Loading

biaoli0 left a comment

IgorNadj commented Jul 18, 2022

biaoli0 commented Jul 18, 2022

Yarn v3 upgrade fixes #4029

Yarn v3 upgrade fixes #4029

Conversation

IgorNadj commented Jul 14, 2022 • edited Loading

biaoli0 left a 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

IgorNadj commented Jul 15, 2022 • edited Loading

biaoli0 left a comment

Choose a reason for hiding this comment

IgorNadj commented Jul 18, 2022

biaoli0 commented Jul 18, 2022

IgorNadj commented Jul 14, 2022 •

edited

Loading

IgorNadj commented Jul 15, 2022 •

edited

Loading