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

[Jest 27 Async Transformer] Runtime not calling correct transform function #11226

Closed
axe-me opened this issue Mar 22, 2021 · 9 comments
Closed

Comments

@axe-me
Copy link

axe-me commented Mar 22, 2021

🐛 Bug Report

Expect the runtime to call transformAsync instead of transform when the custom transformer only contains a processAsync function.

To Reproduce

Steps to reproduce the behavior:

  • git clone git@github.com:axe-me/vite-jest.git
  • run pnpm install at root dir
  • run pnpm build to build the transformer
  • go to the examples folder and run pnpm install
  • run pnpm test
  • you will see some error log like:
> examples@1.0.0 test
> jest

 FAIL  tests/App.test.ts
  ● Test suite failed to run

    Jest: synchronous transformer /Users/Axe/Projects/oss/vite-jest/dist/index.js must export a "process" function.

      at invariant (node_modules/.pnpm/@jest/transform@27.0.0-next.5/node_modules/@jest/transform/build/ScriptTransformer.js:1107:11)
      at assertSyncTransformer (node_modules/.pnpm/@jest/transform@27.0.0-next.5/node_modules/@jest/transform/build/ScriptTransformer.js:1113:3)
      at ScriptTransformer.transformSource (node_modules/.pnpm/@jest/transform@27.0.0-next.5/node_modules/@jest/transform/build/ScriptTransformer.js:639:7)
      at ScriptTransformer._transformAndBuildScript (node_modules/.pnpm/@jest/transform@27.0.0-next.5/node_modules/@jest/transform/build/ScriptTransformer.js:786:40)
      at ScriptTransformer.transform (node_modules/.pnpm/@jest/transform@27.0.0-next.5/node_modules/@jest/transform/build/ScriptTransformer.js:843:19)

Test Suites: 1 failed, 1 total
Tests:       0 total
  • Also if you set the collectCoverage: true in jest.config.js, you are expected to see some other error. I believe they are caused by the same reason.

Expected behavior

My processAsync get called instead of error out.

Link to repl or repo (highly encouraged)

https://github.com/axe-me/vite-jest

envinfo

System:
OS: macOS 11.2.2
CPU: (4) x64 Intel(R) Core(TM) i5-4690 CPU @ 3.50GHz
Binaries:
Node: 14.7.0 - ~/.nvm/versions/node/v14.7.0/bin/node
Yarn: 1.22.10 - ~/.nvm/versions/node/v14.7.0/bin/yarn
npm: 7.5.4 - ~/.nvm/versions/node/v14.7.0/bin/npm
npmPackages:
jest: 27.0.0-next.5 => 27.0.0-next.5

One More thing:

Can we also make createTransformer function to be async as well, if you look into my transformer in the repo you can see I have to put in some hack to create my async transformer.

@axe-me
Copy link
Author

axe-me commented Mar 22, 2021

Another thing:

The current processAsync implementation seems really redundant, do you really need to have an explicit processAsync function at all? Can we just use the same process function and allow it returns a Promise, then just do await process() anywhere invoking this function. This should also have backward compatibility since await won't do anything if the process not returns a Promise.

export interface Transformer<OptionType = unknown> {
    canInstrument?: boolean;
    createTransformer?: (options?: OptionType) => AsyncTransformer<OptionType> | Promise<AsyncTransformer<OptionType>>;
    getCacheKey?: (sourceText: string, sourcePath: Config.Path, options: TransformOptions<OptionType>) => string | Promise<string>;
    process?: (sourceText: string, sourcePath: Config.Path, options: TransformOptions<OptionType>) => TransformedSource | Promise<TransformedSource>;
}

^^^ This is what I expected, thoughts? @SimenB

@SimenB
Copy link
Member

SimenB commented Mar 22, 2021

Async transformations are only supported in ESM mode, which is not activated in your case

https://jestjs.io/docs/en/ecmascript-modules

Doing NODE_OPTIONS=--experimental-vm-modules pnpm test throws

image

Removing that import triggered some other dependency error, so I stopped digging.


Async createTransformer is certainly something we can do, please open up a dedicated feature request. Or better yet, a PR.


Explicit Async variant is to avoid overloading functions. And since CJS cannot use async transforms, having that function retain its signature seems cleaner.

@axe-me
Copy link
Author

axe-me commented Mar 25, 2021

@SimenB Thanks for helping.

I added that Node option and removed the tailwind stuff, now the transformer runs. However I got another error from jest-resolve:

image

And re async createTransformer I will make a PR this weekend.

@SimenB
Copy link
Member

SimenB commented Mar 25, 2021

That doesn't seem related to a transformer but rather a resolver issue. Does not seem like a bug in Jest, though

@axe-me
Copy link
Author

axe-me commented Mar 31, 2021

after looked into it yesterday. I realized this is indeed not a bug in jest, it was Vite transform the import path to the module id with / prefix which caused jest-resolve to errored out since it's not a valid absolute path.
This can only be solved by using a custom resolver to let vite do the module resolving. However again this require the resolver option to be async. So this is blocked by #9505

@SimenB
Copy link
Member

SimenB commented Mar 31, 2021

Implementing #9505 should be relatively straightforward if you're up for it 🙂

But I'll close this as it's not a bug like you say

@SimenB SimenB closed this as completed Mar 31, 2021
@axe-me
Copy link
Author

axe-me commented Mar 31, 2021

I can try to implement it, altho this gonna be the first time I contribute to jest project, I gonna try my best.

@SimenB
Copy link
Member

SimenB commented Mar 31, 2021

👍

Adding support in jest-resolve first makes sense to me, then we can make Jest use that in a separate PR later (similar to what we did for async code transforms).

Feel free to ask questions over in #9505 (and/or open a Draft PR and we can discuss there)

@github-actions
Copy link

This issue 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 May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants