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.

Sign up for GitHub

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

feat: Add support for jest@27 #123

Merged
merged 4 commits into from
Jun 14, 2021
Merged

feat: Add support for jest@27 #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4284af6
chore: upgrade dependencies
pmdartus Jun 9, 2021
ac415f3
Update to jest@27
pmdartus Jun 9, 2021
780fab1
Add matrix test
pmdartus Jun 9, 2021
c6ef0c9
Update command to override jest version
pmdartus Jun 9, 2021
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
61 changes: 30 additions & 31 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
version: 2.1

supported-jest-versions: &supported-jest-versions ["local", "26", "27"]
Copy link
Contributor

Choose a reason for hiding this comment

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

One unfortunate thing about this is that our tests could randomly start failing because of a change introduced in a minor/patch release of Jest 26 or 27. Some alternatives I can think of:

  1. Explicitly spell out the versions here (e.g. ["local", "26.6.3", "27.0.4"])
  2. Add additional devDependencies like so:
  "devDependencies": {
    "jest": "^27.0.4",
    "jest-26": "npm:jest@^26",
    "jest-27": "npm:jest@^27"
  }

Then we would need to do something like JEST_BIN=./node_modules/jest-26/bin/jest.js to switch between them.

I slightly prefer the second option, because the versions would go in the yarn lockfile and therefore would be part of Circle CI's yarn cache. Also it would be easier to remember to update these versions, rather than keeping them in .circleci/config.yml.

On the other hand, the second option is more complex and unusual. So I could go either way.

Copy link
Member

Choose a reason for hiding this comment

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

One unfortunate thing about this is that our tests could randomly start failing because of a change introduced in a minor/patch release of Jest 26 or 27.

Sounds like a feature to me! It would be great to run tests on every jest release but perhaps installing latest per run is the next best thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is the intended behavior. Because we are using ^ to match against the jest peer dependency version, we don't have much control over the version of jest consumers' uses.

Ideally, we would need to run the test every time a new version of jest is published to catch bugs earlier. But it would be overengineering this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's what I'm concerned about:

  1. This project isn't updated for a few months
  2. Someone opens a PR
  3. The PR's tests are broken in CI
  4. Now we have to spend time figuring out whether the PR broke things or a Jest update broke things

If we really want to stay on top of Jest updates, the solution IMO is something like Dependabot, not "random breakages when someone opens a PR or clones the projects and tries to test it." That doesn't even solve the problem, because the update cadence aligns with updates to this project, not updates to Jest.

To be fair, though, I don't know if Dependabot is smart enough to understand the npm:package@version syntax. So this problem may be unsolveable.

Copy link
Member

Choose a reason for hiding this comment

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

@nolanlawson Agreed. Maybe we can add jest updates to the list of tasks for the weekly trust hero?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now we have to spend time figuring out whether the PR broke things or a Jest update broke things

This is why there are 3 different builds in the matrix: "local", "26", "27"

  • The local build checks if the jest version frozen in the lockfile pass the tests
  • The 26 and 27 builds checks if the latest versions of jest pass the tests

If the local build pass and either 26 or 27 fails, it means that there is a regression or a bug in the latest version of Jest.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, fair enough.


deploy_filters: &deploy_filters
filters:
Expand Down Expand Up @@ -33,47 +34,47 @@ commands:
keys:
- yarn-v5-{{ checksum "yarn.lock" }}

save_workspace:
description: Save current workspace
steps:
- persist_to_workspace:
root: .
paths: .

load_workspace:
description: Load workspace
steps:
- attach_workspace:
at: ~/lwc-test


jobs:
build:
parameters:
jest-version:
type: string
description: >
Overrides the installed version of jest. When set to "local" the version of jest set
frozen in the yarn.lock is used.
default: "local"
executor: node
steps:
- checkout
- restore_yarn_cache
- run:
name: Install dependencies and build
command: yarn install --frozen-lockfile
- save_yarn_cache
- when:
condition:
not:
equal: [<<parameters.jest-version>>, "local"]
steps:
- run:
name: Override version of jest@<<parameters.jest-version>>
command: yarn add jest@<<parameters.jest-version>> -W --dev
- run:
name: Run linter
command: yarn lint
- save_yarn_cache
- save_workspace

test:
executor: node
steps:
- load_workspace
- run:
name: Run unit tests
command: yarn test --runInBand

deploy:
executor: node
steps:
- load_workspace
- checkout
- restore_yarn_cache
- run:
name: Install dependencies and build
command: yarn install --frozen-lockfile
- run:
name: Configure NPM authentication
command: npm config set "//registry.npmjs.org/:_authToken" "$NPM_AUTOMATION_TOKEN"
Expand All @@ -84,22 +85,20 @@ jobs:
workflows:
build_and_test:
jobs:
- build
- test:
requires:
- build
- build:
matrix:
parameters:
jest-version: *supported-jest-versions

build_and_test_and_deploy:
jobs:
- build:
<<: *deploy_filters

- test:
<<: *deploy_filters
requires:
- build
matrix:
parameters:
jest-version: *supported-jest-versions

- deploy:
<<: *deploy_filters
requires:
- test
- build
68 changes: 3 additions & 65 deletions .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,77 +1,15 @@
{
"extends": "eslint:recommended",
"parser": "babel-eslint",
"parser": "@babel/eslint-parser",
"parserOptions": {
"ecmaVersion": 9,
"sourceType": "module"
"sourceType": "module",
"requireConfigFile": false
},
"env": {
"es6": true,
"browser": true,
"jest": true,
"node": true
},
"rules": {
"block-scoped-var": "error",
"no-alert": "warn",
"no-buffer-constructor": "error",
"no-eval": "error",
"no-extra-bind": "error",
"no-extra-label": "error",
"no-iterator": "error",
"no-lone-blocks": "error",
"no-proto": "error",
"no-new-require": "error",
"no-restricted-properties": ["error", {
"object": "arguments",
"property": "callee",
"message": "arguments.callee is deprecated"
}, {
"object": "global",
"property": "isFinite",
"message": "Please use Number.isFinite instead"
}, {
"object": "self",
"property": "isFinite",
"message": "Please use Number.isFinite instead"
}, {
"object": "window",
"property": "isFinite",
"message": "Please use Number.isFinite instead"
}, {
"object": "global",
"property": "isNaN",
"message": "Please use Number.isNaN instead"
}, {
"object": "self",
"property": "isNaN",
"message": "Please use Number.isNaN instead"
}, {
"object": "window",
"property": "isNaN",
"message": "Please use Number.isNaN instead"
}, {
"property": "__defineGetter__",
"message": "Please use Object.defineProperty instead."
}, {
"property": "__defineSetter__",
"message": "Please use Object.defineProperty instead."
}, {
"object": "Math",
"property": "pow",
"message": "Use the exponentiation operator (**) instead."
}],
"no-self-compare": "error",
"no-undef-init": "error",
"no-useless-catch": "error",
"no-useless-computed-key": "error",
"no-useless-return": "error",
"no-with": "error",
"prefer-const": ["error", {
"destructuring": "any",
"ignoreReadBeforeAssign": true
}],
"template-curly-spacing": "error",
"yoda": "error"
}
}
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@
"format": "prettier --write '{packages,test}/**/*.{js,ts,json,md}'",
"release:publish:ci": "./scripts/release/publish.js",
"release:version": "./scripts/release/version.js"

},
"workspaces": [
"packages/@lwc/*",
"test"
],
"license": "MIT",
"devDependencies": {
"@babel/core": "^7.14.3",
"@babel/eslint-parser": "^7.14.4",
"@lwc/compiler": "^2.1.0",
"@lwc/engine-dom": "^2.1.0",
"@lwc/synthetic-shadow": "^2.1.0",
"@lwc/wire-service": "^2.1.0",
"@types/jest": "^26.0.21",
"babel-eslint": "^10.1.0",
"eslint": "^7.22.0",
"jest": "^26.6.3",
"jest": "^27.0.4",
"lerna": "^4.0.0",
"prettier": "^2.2.1"
}
Expand Down
3 changes: 3 additions & 0 deletions packages/@lwc/jest-preset/jest-preset.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
module.exports = {
// Starting jest@27, the default test environment is "node". Force test environment to "jsdom"
// regardless of the jest version.
testEnvironment: 'jsdom',
moduleFileExtensions: ['ts', 'js', 'html'],
resolver: require.resolve('@lwc/jest-resolver'),
transform: {
Expand Down
2 changes: 1 addition & 1 deletion packages/@lwc/jest-preset/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"@lwc/compiler": "*",
"@lwc/engine-dom": "*",
"@lwc/synthetic-shadow": "*",
"jest": "^26"
"jest": "^26 || ^27"
},
"dependencies": {
"@lwc/jest-resolver": "11.0.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/@lwc/jest-resolver/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"/src/index.js"
],
"peerDependencies": {
"jest": "^26"
"jest": "^26 || ^27"
},
"engines": {
"node": ">=10"
Expand Down
4 changes: 2 additions & 2 deletions packages/@lwc/jest-serializer/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
"lwc"
],
"dependencies": {
"pretty-format": "^26.6.2"
"pretty-format": "^27.0.2"
},
"peerDependencies": {
"jest": "^26"
"jest": "^26 || ^27"
},
"engines": {
"node": ">=10"
Expand Down
4 changes: 2 additions & 2 deletions packages/@lwc/jest-transformer/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
"@babel/plugin-syntax-decorators": "^7.12.13",
"@babel/plugin-transform-modules-commonjs": "^7.13.8",
"@babel/preset-typescript": "^7.13.0",
"babel-preset-jest": "^26.6.2"
"babel-preset-jest": "^27.0.1"
},
"peerDependencies": {
"@lwc/compiler": "*",
"jest": "^26"
"jest": "^26 || ^27"
},
"engines": {
"node": ">=10"
Expand Down
25 changes: 22 additions & 3 deletions packages/@lwc/jest-transformer/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,31 @@ module.exports = {

return babelCore.transform(code, { ...BABEL_CONFIG, ...config, filename });
},
getCacheKey(fileData, filePath, configStr, options) {

getCacheKey(sourceText, sourcePath, ...rest) {
let configString;
let transformConfig;

if (rest.length === 1) {
// Handle jest@27 arguments
// type getCacheKey = (sourceText: string, sourcePath: string, options: { configString: string }) => string;
transformConfig = rest[0];
configString = transformConfig.configString;
} else if (rest.length === 2) {
// Handle jest@26 arguments
// type getCacheKey = (sourceText: string, sourcePath: string, configStr: string, options: any) => string;
configString = rest[0];
transformConfig = rest[1];
} else {
throw new Error('Unexpected transform arguments.');
}

const { NODE_ENV } = process.env;

return crypto
.createHash('md5')
.update(JSON.stringify(options), 'utf8')
.update(fileData + filePath + configStr + NODE_ENV + compilerVersion, 'utf8')
.update(JSON.stringify(configString), 'utf8')
.update(sourceText + sourcePath + configString + NODE_ENV + compilerVersion, 'utf8')
.digest('hex');
},
};
Loading