Skip to content

refactor(jest): make jest presets version-specific#4904

Merged
rwaskiewicz merged 1 commit intomainfrom
rwaskiewicz/split-jest-preset
Oct 11, 2023
Merged

refactor(jest): make jest presets version-specific#4904
rwaskiewicz merged 1 commit intomainfrom
rwaskiewicz/split-jest-preset

Conversation

@rwaskiewicz
Copy link
Member

@rwaskiewicz rwaskiewicz commented Oct 6, 2023

Note to Reviewers

Depends on #4883, please review but do not merge

What is the current behavior?

Our existing Jest preset is fixed. That is, it assumes that it will be static/unchanged regardless of the version of Jest that a user uses. This PR seeks to change that

GitHub Issue Number: N/A

What is the new behavior?

this commit updates the existing infrastructure for jest presets to move
them under version-specific directories (although there's just one such
directory at this time).

in doing so, the jest-preset.js file that was previously housed under
the scripts/ directory now has the same dispatch behavior as the other
javascript files in the same directory (git mv was used to preserve
history of the original contents). this allows the correct version of
jest-preset to be selected at runtime when a user invokes jest.

jest-preset.ts contains the original preset with the following
changes:

  1. it no longer uses CommonJS exports to make loading the file at
    runtime easier and compile-time compliant. it is worth noting that
    the contents of this file, in their previous location, would simply
    be copied to the output directory. now, they are run through the
    typescript compiler and placed in testing/index.js after the
    compiler has been built
  2. path is now imported with an import statement, rather than a
    require statement as a consequence of point 1.

the resulting directory structure looks something like:

  ├── scripts/bundles/helpers/jest/
  │   └── jest-preset.js                      <- Now has same dispatch behavior as other files in this directory
  ├── src/testing/jest/
  │   └── jest-27-and-under
  │       ├── jest-config.ts
  │       ├── jest-environment.ts
  │       ├── jest-facade.ts
  │       ├── jest-preprocessor.ts
+ │       ├── jest-preset.ts                  <- Contains preset previously in `scripts/`, now as a module
  │       ├── jest-runner.ts
  │       ├── jest-screenshot.ts
  │       ├── jest-serializer.ts
  │       ├── jest-setup-test-framework.ts
  │       ├── node_modules/
  │       ├── package.json
  │       └── test
  ├── jest-adapter.ts
  ├── jest-apis.ts
  ├── jest-stencil-connector.ts
  └── package.json

the jest adapter and it's related classes have been updated to return
the correct jest preset

Does this introduce a breaking change?

  • Yes
  • No

Testing

Pull down this branch and build it. Make note of the location of the generated tarball:

npm run clean \                   
&& npm ci \
&& npm run build \
&& npm pack

Next, create a new stencil component library with that tarball:

cd /tmp \
&& npm init stencil@latest component jest-test \
&& cd $_ \
&& npm i [PATH_TO_TARBALL]

Stencil Testing should continue to pass for Jest [24,27]:

npm i jest@24 jest-cli@24 @types/jest@24 && npm t -- --no-cache && \
npm i jest@25 jest-cli@25 @types/jest@25 && npm t -- --no-cache && \
npm i jest@26 jest-cli@26 @types/jest@26 && npm t -- --no-cache && \
npm i jest@27 jest-cli@27 @types/jest@27 && npm t -- --no-cache

Other information

I recommended walking the commits in this PR to review. They better illustrate the git mv step and the changes made. Although the summation of the diffs is something like +74/-39, the 'show me all changes in this PR view' makes this look far worse than it actually is.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 1399 errors on this branch.

That's the same number of errors on main, so at least we're not creating new ones!

reports and statistics

Our most error-prone files
Path Error Count
src/dev-server/index.ts 37
src/mock-doc/serialize-node.ts 36
src/dev-server/server-process.ts 32
src/compiler/build/build-stats.ts 27
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts 25
src/compiler/style/test/optimize-css.spec.ts 23
src/testing/puppeteer/puppeteer-element.ts 23
src/compiler/prerender/prerender-main.ts 22
src/runtime/vdom/vdom-render.ts 20
src/runtime/client-hydrate.ts 19
src/screenshot/connector-base.ts 19
src/compiler/config/test/validate-paths.spec.ts 16
src/dev-server/request-handler.ts 15
src/compiler/prerender/prerender-optimize.ts 14
src/compiler/sys/stencil-sys.ts 14
src/compiler/transpile/transpile-module.ts 14
src/runtime/vdom/vdom-annotations.ts 14
src/sys/node/node-sys.ts 14
src/compiler/build/build-finish.ts 13
src/compiler/prerender/prerender-queue.ts 13
Our most common errors
Typescript Error Code Count
TS2345 424
TS2322 398
TS18048 310
TS18047 100
TS2722 38
TS2532 34
TS2531 23
TS2454 14
TS2352 13
TS2769 10
TS2790 10
TS2538 8
TS2344 5
TS2416 4
TS2493 3
TS18046 2
TS2684 1
TS2488 1
TS2430 1

Unused exports report

There are 12 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports
File Line Identifier
src/runtime/bootstrap-lazy.ts 21 setNonce
src/screenshot/screenshot-fs.ts 18 readScreenshotData
src/testing/testing-utils.ts 198 withSilentWarn
src/utils/index.ts 140 CUSTOM
src/compiler/app-core/app-data.ts 25 BUILD
src/compiler/app-core/app-data.ts 114 Env
src/compiler/app-core/app-data.ts 116 NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts 123 updateCacheFromRebuild
src/compiler/types/validate-primary-package-output-target.ts 62 satisfies
src/compiler/types/validate-primary-package-output-target.ts 62 Record
src/testing/puppeteer/puppeteer-declarations.ts 485 WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts 7 writeFetchSuccessSync

@rwaskiewicz rwaskiewicz marked this pull request as ready for review October 6, 2023 12:59
@rwaskiewicz rwaskiewicz requested a review from a team as a code owner October 6, 2023 12:59
@rwaskiewicz rwaskiewicz requested review from alicewriteswrongs, christian-bromann and tanner-reits and removed request for a team October 6, 2023 12:59
Copy link
Contributor

@tanner-reits tanner-reits left a comment

Choose a reason for hiding this comment

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

One non-blocker. Haven't ran any of the test rework stuff locally yet, but I'll do that once I'm out of PR review hell

const moduleExtensions = ['ts', 'tsx', 'js', 'mjs', 'jsx'];
const moduleExtensionRegexp = '(' + moduleExtensions.join('|') + ')';

const preset: Config.InitialOptions = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the JestConfig type you made?

Copy link
Member Author

Choose a reason for hiding this comment

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

The JestConfig type is actually going to be changing in the near-ish future when we add support for v28+ to be wider than just the typings of Jest 27. By using Config.InitialOptions here, we guarantee that this preset adheres to Jest 27's configuration shape, rather than one that (will) be a combination of 27 + 28 + 29

@rwaskiewicz rwaskiewicz force-pushed the rwaskiewicz/split-jest-preset branch 2 times, most recently from 988c88e to a451686 Compare October 10, 2023 14:02
Copy link
Contributor

@alicewriteswrongs alicewriteswrongs left a comment

Choose a reason for hiding this comment

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

tried it out locally, all looks good!

@rwaskiewicz rwaskiewicz force-pushed the rwaskiewicz/split-jest-preset branch from a451686 to 0f00633 Compare October 11, 2023 17:26
@rwaskiewicz rwaskiewicz force-pushed the rwaskiewicz/jest/step-2-get-version branch from 4327cfc to 8dc8895 Compare October 11, 2023 19:56
Base automatically changed from rwaskiewicz/jest/step-2-get-version to main October 11, 2023 20:18
this commit updates the existing infrastructure for jest presets to move
them under version-specific directories (although there's just one such
directory at this time).

in doing so, the `jest-preset.js` file that was previously housed under
the `scripts/` directory now has the same dispatch behavior as the other
javascript files in the same directory (`git mv` was used to preserve
history of the original contents). this allows the correct version of
`jest-preset` to be selected at runtime when a user invokes jest.

`jest-preset.ts` contains the original preset with the following
changes:
1. it no longer uses CommonJS exports to make loading the file at
   runtime easier and compile-time compliant. it is worth noting that
   the contents of this file, in their previous location, would simply
   be copied to the output directory. now, they are run through the
   typescript compiler and placed in `testing/index.js` after the
   compiler has been built
2. `path` is now imported with an `import` statement, rather than a
   `require` statement as a consequence of point 1.

the resulting directory structure looks something like:

```diff
  ├── scripts/bundles/helpers/jest/
  │   └── jest-preset.js                      <- Now has same dispatch behavior as other files in this directory
  ├── src/testing/jest/
  │   └── jest-27-and-under
  │       ├── jest-config.ts
  │       ├── jest-environment.ts
  │       ├── jest-facade.ts
  │       ├── jest-preprocessor.ts
+ │       ├── jest-preset.ts                  <- Contains preset previously in `scripts/`, now as a module
  │       ├── jest-runner.ts
  │       ├── jest-screenshot.ts
  │       ├── jest-serializer.ts
  │       ├── jest-setup-test-framework.ts
  │       ├── node_modules/
  │       ├── package.json
  │       └── test
  ├── jest-apis.ts
  ├── jest-facade.ts
  ├── jest-stencil-connector.ts
  └── package.json
```

the jest facade interface and it's related impl have been updated to return
the correct jest preset
@rwaskiewicz rwaskiewicz force-pushed the rwaskiewicz/split-jest-preset branch from ef992d5 to 4e8979d Compare October 11, 2023 20:38
@rwaskiewicz rwaskiewicz enabled auto-merge October 11, 2023 20:40
@rwaskiewicz rwaskiewicz added this pull request to the merge queue Oct 11, 2023
Merged via the queue into main with commit 6893954 Oct 11, 2023
@rwaskiewicz rwaskiewicz deleted the rwaskiewicz/split-jest-preset branch October 11, 2023 21:02
rwaskiewicz added a commit that referenced this pull request Oct 24, 2023
copy the contents of `jest-27-and-under/` to a new `jest/` subdirectory,
`jest-28`. this directory contains an exact replica of
`jest-27-and-under/` as of 6893954 (#4904).

note that since then, eb3ae9a (#4950) has landed. this is handled in a
subesquent commit
rwaskiewicz added a commit that referenced this pull request Oct 24, 2023
copy the contents of `jest-27-and-under/` to a new `jest/` subdirectory,
`jest-28`. this directory contains an exact replica of
`jest-27-and-under/` as of 6893954 (#4904).

note that since then, eb3ae9a (#4950) has landed. this is handled in a
subesquent commit
rwaskiewicz added a commit that referenced this pull request Oct 26, 2023
add suport for jest 28 in stencil.

this commit is the first pass at support a new version of jest in
stencil in well over a year, and the first to use the new package-based
architecture.

this commit was generated using the steps that are outline in the README
file of the jest directory, [available here](https://github.com/ionic-team/stencil/blob/d725ac9ef73f9700feb773d4a65e3ea988db6d31/src/testing/jest/README.md?plain=1).

note: some commits of the original pr may be out of order with the
readme above. although the readme is permalinked, it did undergo some
changes between the time this pr's commits were created and the time this
pr was created. some items of note:
- the contents of `jest-27-and-under/` were copied into `jest-28/` using
  the latest available contents at the time - 6893954 (#4904).

in order to support jest 28, the following changes were made:
1. projects are now typed as `string` in v28:
   the typings for jest are causing the project to fail compilation. update
   the return value here, as `Config.Path` is now just a string
1. resolve TS4053 error in v28 facade:
   we had to resolve a typescript compilation error where we would run
   into typescript error TS4053. this was caused by this facade's
   implementation of `getJestPreset` returning a Jest preset that
   referenced Jest v28's `Config` interface. the TypeScript compiler did
   not know how to make sense of this import within the module, requiring
   us to manually import it
1. update typings of the preprocessor:
   in jest 28, preprocessors cannot return a string. rather, they must at
   minimum return `{ code : string }`. this commit resolves those
   compilation errors by using Jest's `TransformedSource` type and updating
   the return values of the function accordingly
1. migrate to jest-circus for v28:
   switch users over to jest-circus for v28, rather than jest-jasmine2.
   this makes it such that we are not required to force users to add
   jest-jasmine2 to their dependencies, since it was removed from jest in
   v28 (and published as a separate package).
1. handle updated environment export:
   the test environments export has changed in v28. update the require
   statement to handle this change.
1. remove jasmine globals:
   remove direct references to jasmine globals that were used in jest 28
   code. in some instnaces, puppeteer code that is not versioned (per
   version of jest) had to be modified to ensure that we did not attempt to
   reference a global `jasmine` that did not exist

finally, some backwards compatability code was removed from the new
`jest-28/` directory. it did not need to be executed in this new
architecture (and can be siloed to `jest-27-and-under/`)

closes: 3348

STENCIL-955
rwaskiewicz added a commit that referenced this pull request Oct 26, 2023
copy the contents of `jest-27-and-under/` to a new `jest/` subdirectory,
`jest-28`. this directory contains an exact replica of
`jest-27-and-under/` as of 6893954 (#4904).

note that since then, eb3ae9a (#4950) has landed. this is handled in a
subesquent commit
github-merge-queue bot pushed a commit that referenced this pull request Oct 26, 2023
add suport for jest 28 in stencil.

this commit is the first pass at support a new version of jest in
stencil in well over a year, and the first to use the new package-based
architecture.

this commit was generated using the steps that are outline in the README
file of the jest directory, [available here](https://github.com/ionic-team/stencil/blob/d725ac9ef73f9700feb773d4a65e3ea988db6d31/src/testing/jest/README.md?plain=1).

note: some commits of the original pr may be out of order with the
readme above. although the readme is permalinked, it did undergo some
changes between the time this pr's commits were created and the time this
pr was created. some items of note:
- the contents of `jest-27-and-under/` were copied into `jest-28/` using
  the latest available contents at the time - 6893954 (#4904).

in order to support jest 28, the following changes were made:
1. projects are now typed as `string` in v28:
   the typings for jest are causing the project to fail compilation. update
   the return value here, as `Config.Path` is now just a string
1. resolve TS4053 error in v28 facade:
   we had to resolve a typescript compilation error where we would run
   into typescript error TS4053. this was caused by this facade's
   implementation of `getJestPreset` returning a Jest preset that
   referenced Jest v28's `Config` interface. the TypeScript compiler did
   not know how to make sense of this import within the module, requiring
   us to manually import it
1. update typings of the preprocessor:
   in jest 28, preprocessors cannot return a string. rather, they must at
   minimum return `{ code : string }`. this commit resolves those
   compilation errors by using Jest's `TransformedSource` type and updating
   the return values of the function accordingly
1. migrate to jest-circus for v28:
   switch users over to jest-circus for v28, rather than jest-jasmine2.
   this makes it such that we are not required to force users to add
   jest-jasmine2 to their dependencies, since it was removed from jest in
   v28 (and published as a separate package).
1. handle updated environment export:
   the test environments export has changed in v28. update the require
   statement to handle this change.
1. remove jasmine globals:
   remove direct references to jasmine globals that were used in jest 28
   code. in some instnaces, puppeteer code that is not versioned (per
   version of jest) had to be modified to ensure that we did not attempt to
   reference a global `jasmine` that did not exist

finally, some backwards compatability code was removed from the new
`jest-28/` directory. it did not need to be executed in this new
architecture (and can be siloed to `jest-27-and-under/`)

closes: 3348

STENCIL-955
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants