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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support transforming all modules which can be specified in config #8810

Open
Mark1626 opened this issue Aug 11, 2019 · 22 comments
Open

Support transforming all modules which can be specified in config #8810

Mark1626 opened this issue Aug 11, 2019 · 22 comments

Comments

@Mark1626
Copy link
Contributor

Mark1626 commented Aug 11, 2019

馃殌 Feature Proposal

Currently transform only converts tests files and some of the configurable modules, allows all modules which can be configured to be transformed

Based on a discussion in #8330 from @SimenB @G-Rath

List pointed out be @G-Rath

Modules which are transformed:

  • snapshotSerializers
  • setupFiles
  • setupFilesAfterEnv
  • globalSetup
  • globalTeardown

Modules which not transformed:

Motivation

Approach will be similar to #8751

Reference #8756

Example

// jest.config.js
module.exports = {
  testEnvironment: 'environment.ts',
  runner: 'runner.ts',
  transform: {
    '^.+\\.ts?$':  'my-transform'
  }
}
@Mark1626
Copy link
Contributor Author

Some of the modules in the list might require #8808 to be merged

@G-Rath
Copy link
Contributor

G-Rath commented Aug 11, 2019

@M4rk9696 You've bet me to it XD

I'm happy to tackle all of them, as I can sit down and knock them all out one after each other, but obviously won't steal any from you :P

@SimenB I' m also happy to help make things async where possible :)

I apparently forgot the whole "make transformer sync" PR, so ignore my async comments 馃槵

I've done some more checking, and the changes need to happen here:


resolver via jest-resolve:

https://github.com/facebook/jest/blob/d9b43a88bf9c5d4eb3e12b88edad60ac1ee30609/packages/jest-resolve/src/index.ts#L90-L92

@SimenB This'll need to be made async


dependencyExtractor via jest-haste-map:

https://github.com/facebook/jest/blob/d9b43a88bf9c5d4eb3e12b88edad60ac1ee30609/packages/jest-haste-map/src/index.ts#L297-L302

@SimenB this'll need to be made async, and be tricky b/c it's in the constructor


testSequencer is an interesting one. It's done in jest-config:

https://github.com/facebook/jest/blob/d9b43a88bf9c5d4eb3e12b88edad60ac1ee30609/packages/jest-config/src/utils.ts#L240-L250

But uses findNodeModule, so it'll need that to be made to somehow get a usable transformer async first.

@SimenB also needs to be made async.

testResultsProcessor is in jest-core:

https://github.com/facebook/jest/blob/d9b43a88bf9c5d4eb3e12b88edad60ac1ee30609/packages/jest-core/src/runJest.ts#L99-L101

@SimonB it'll need to be made async too

(I'll find the other spots once I'm at work in an hour or so)

@Mark1626
Copy link
Contributor Author

Mark1626 commented Aug 11, 2019

Picking up runner

runner via jest-core

https://github.com/facebook/jest/blob/86e73f5b22e8a02b5233af78c68ef7318c59e1b3/packages/jest-core/src/TestScheduler.ts#L174

testRunner via jest-runner

https://github.com/facebook/jest/blob/d9b43a88bf9c5d4eb3e12b88edad60ac1ee30609/packages/jest-runner/src/runTest.ts#L111-L113

@SimenB Types required for test-runner are not exported, noticed it when creating the e2e will raise a separate PR for that
@G-Rath Think the other modules will also need types exported

@G-Rath
Copy link
Contributor

G-Rath commented Aug 11, 2019

On a side note; is @jest/types the new @types/jest (yet*)?

I've not explored enough to compare the two, but they've got more types - snapshotResolver iirc isn't in @types for example.

Just wondering if you've consumed that at some point.

*: At some point they will for sure, so I mean right now

@G-Rath
Copy link
Contributor

G-Rath commented Aug 11, 2019

Also here's another spanner in the works: ScriptTransformer requires config, which typically most things don't have at point-of-require

Maybe it's worth making a global ScriptTransformer somehow..?

I'm just thinking of the fact that findNodeModule in jest-resolve is static and seems to be for general use, and doesn't get passed the entire config, so it'll be a pretty big change at this point.

That's required for at least two options to support transforming

@SimenB
Copy link
Member

SimenB commented Aug 12, 2019

@jest/types has absolutely zero relation to @types/jest and never will. When we start exporting types for consumers of jest, it'll be in the jest package.


I'm just thinking of the fact that findNodeModule in jest-resolve is static and seems to be for general use, and doesn't get passed the entire config, so it'll be a pretty big change at this point.

Why? It just resolves a file path, it doesn't actually require anything

testSequencer is an interesting one. It's done in jest-config:

Same as above that's just resolving a path, not actually doing require


Also here's another spanner in the works: ScriptTransformer requires config, which typically most things don't have at point-of-require

Everywhere we do require should have it - if not just pass it in.

Maybe it's worth making a global ScriptTransformer somehow..?

No, we don't want to share state

@G-Rath
Copy link
Contributor

G-Rath commented Aug 12, 2019

@jest/types has absolutely zero relation to @types/jest and never will.

Noted - I'll make to put in a PR to @types so that this work can be useable :)

Why? It just resolves a file path, it doesn't actually require anything

Line 91:

https://github.com/facebook/jest/blob/d9b43a88bf9c5d4eb3e12b88edad60ac1ee30609/packages/jest-resolve/src/index.ts#L86-L93

Everywhere we do require should have it - if not just pass it in.

Cool, just wanting to make sure I'm not overlooking anything, or that people will be surprised if suddenly a bunch of parameters change :)

No, we don't want to share state

Agreed

@SimenB
Copy link
Member

SimenB commented Aug 12, 2019

Ah, the resolver itself. That might not be easy regardless of having access to ProjectConfig or not as it's done during bootstrap. We can just hold off on it

@Mark1626
Copy link
Contributor Author

Mark1626 commented Aug 14, 2019

Working on runner right now, seems that I need to the type in the signature

async runTests(
    tests: Array<Test>,
    watcher: TestWatcher,
    onStart: OnTestStart,
    onResult: OnTestSuccess,
    onFailure: OnTestFailure,
    options: TestRunnerOptions,
  ): Promise<void>

I'm planning on importing with a different alias and exporting it within the namespace TestRunner, similar to the following

https://github.com/facebook/jest/blob/abb760a2614d29a39a6dd062288c847a5776f493/packages/jest-runner/src/index.ts#L32-L34

I'll raise the types as a separate PR
Any comments?

@jeysal
Copy link
Contributor

jeysal commented May 30, 2020

I've added {coverageR,r}eporters to the list after #10105 was opened. If there's any reason why those would not be included please do shout anyone 馃檭

@G-Rath
Copy link
Contributor

G-Rath commented Sep 30, 2020

@jeysal (or someone who can edit the issue): could you add globalTeardown to list?

Please & thank you :)

@Bnaya
Copy link

Bnaya commented Sep 30, 2020

A possible workaround, point to a JS file that register transpiler and require the source file

// forSetupFilesAfterEnv.js
const tsNode = require('ts-node');

tsNode.register({
  transpileOnly: true,
  compilerOptions: require('@testim/root-cause-jest/tsconfig').compilerOptions,
});

try {
  require('@testim/root-cause-jest/lib/forSetupFilesAfterEnv');
} catch (e) {
  console.error(e);
  throw e;
}

@SimenB
Copy link
Member

SimenB commented Oct 23, 2020

@G-Rath @Mark1626 I'll start landing these next week. So if you wanna send PRs for even more of them, now's a perfect time 馃槆

@Mark1626
Copy link
Contributor Author

Thanks, going through all the conversation to revisit the context.

I'm starting to look at the code for test-sequencer

Should I wait for existing ones to be merged before landing new ones to reduce the effort of resolving merge conflicts?

@SimenB
Copy link
Member

SimenB commented Nov 14, 2020

@Mark1626 yeah, probably. I'll try to find the time to rebase and land the open ones tomorrow. If you have time to rebase them that'd be a great help 馃憤

@Mark1626
Copy link
Contributor Author

@SimenB Rebased all the three open PRs jest-env, jest-runner, jest-testrunner, CI seems to be failing for macOS builds(not sure why seems to fail for me in master as well), in test-runner yarn lock needs to be updated as dependency on @jest/transform has been added.

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@mnoorenberghe
Copy link

I believe I still saw some files the other day which don't get transformed but I don't recall which.

@github-actions github-actions bot removed the Stale label Feb 27, 2022
@SimenB
Copy link
Member

SimenB commented Feb 27, 2022

resolver is not, I'm not sure about others. If somebody could go through and verify the current state, that'd be awesome 馃檪

@bradydowling
Copy link

Does anyone have a workaround for a custom reporter? cc/ @Xotabu4

@SimenB SimenB added the Pinned label Apr 8, 2022
@SimenB
Copy link
Member

SimenB commented Apr 8, 2022

@treyturner
Copy link

treyturner commented Nov 18, 2022

It should be enough to use ScriptTransformer#requireAndTranspileModule here:

https://github.com/facebook/jest/blob/a5f1ef43981b7426d61bcd7cbc59a79f548c075a/packages/jest-core/src/TestScheduler.ts#L406

. See
https://github.com/facebook/jest/blob/a5f1ef43981b7426d61bcd7cbc59a79f548c075a/packages/jest-core/src/runGlobalHook.ts#L41-L46

for how to create a transformer. Wanna send a PR?

I need this feature and was willing to take a stab at it, but it's not clear to me how I can access allTests from TestScheduler.ts for getting the correct config (or a fallback).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants