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

feat: custom haste #11107

Merged
merged 9 commits into from
May 20, 2021
Merged

feat: custom haste #11107

merged 9 commits into from
May 20, 2021

Conversation

DiZy
Copy link
Contributor

@DiZy DiZy commented Feb 22, 2021

Summary

At Facebook, we want to use a completely custom implementation of the haste map. Overriding individual pieces such as hasteImplModulePath isn't enough to be achieve our goals with the custom implementation. We need many pieces of the build process overwritten: the crawlers, the format it is persisted to disk, and the module map. We also have added dependencies such as a sqlite library.

This change introduces the option to completely override the HasteMap class with a custom implementation. This involves relatively little changes to HasteMap itself, other than some new interfaces. The one added method is "getModuleMapFromJSON" inside HasteMap. This is needed because our custom implementation also overrides the ModuleMap serialization, so we need to go to/from serialized module maps in the correct format.

Test plan

An E2E test was written which overrides the HasteMap using this option. This custom implementation overrides all the pieces we need, including the ModuleMap. We can see with this that modules are able to be resolved with specific names as defined in the custom implementation. We can also see the custom module map serialization/deserialization working.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

left some (hopeful) q's.

(missing changelog entry 😀)

packages/jest-runner/src/testWorker.ts Outdated Show resolved Hide resolved
packages/jest-runtime/src/index.ts Outdated Show resolved Hide resolved
packages/jest-types/src/Config.ts Show resolved Hide resolved
@@ -219,7 +229,22 @@ export default class HasteMap extends EventEmitter {
private _watchers: Array<Watcher>;
private _worker: WorkerInterface | null;

constructor(options: Options) {
static getStatic(config: Config.ProjectConfig): HasteMapStatic {
Copy link
Member

Choose a reason for hiding this comment

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

can we make this async so we can use import(config.haste.hasteMapModulePath) in the future? same for create

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing to async complicates some call sites such as the setup function in testWorker.ts

Copy link
Member

Choose a reason for hiding this comment

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

right, but it needs to be async in order to use ESM. so we either need to make it so now, or make it async whenever somebody wants to write these modules using ESM (such as a WASM implementation)

packages/jest-haste-map/src/types.ts Outdated Show resolved Hide resolved
packages/jest-resolve/src/index.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2021

Codecov Report

Merging #11107 (41287a3) into master (b7c968a) will decrease coverage by 0.03%.
The diff coverage is 35.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11107      +/-   ##
==========================================
- Coverage   64.20%   64.17%   -0.04%     
==========================================
  Files         309      309              
  Lines       13524    13534      +10     
  Branches     3295     3297       +2     
==========================================
+ Hits         8683     8685       +2     
- Misses       4126     4134       +8     
  Partials      715      715              
Impacted Files Coverage Δ
packages/jest-config/src/ValidConfig.ts 100.00% <ø> (ø)
packages/jest-haste-map/src/ModuleMap.ts 56.25% <ø> (ø)
packages/jest-haste-map/src/index.ts 79.06% <0.00%> (-1.89%) ⬇️
packages/jest-resolve/src/index.ts 45.96% <ø> (ø)
packages/jest-runner/src/testWorker.ts 0.00% <0.00%> (ø)
packages/jest-runtime/src/index.ts 53.31% <100.00%> (ø)
packages/jest-test-sequencer/src/index.ts 90.56% <100.00%> (+0.18%) ⬆️
packages/jest-transform/src/ScriptTransformer.ts 73.46% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7c968a...41287a3. Read the comment docs.

@DiZy DiZy marked this pull request as ready for review May 12, 2021 17:33
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

left some nits, and missing changelog. Other than that this LGTM 👍

@SimenB SimenB changed the title Custom haste feat: custom haste May 20, 2021
@SimenB SimenB merged commit 4fa3a0b into jestjs:master May 20, 2021
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants