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

[Bug]: Doesn't fallback to esbuild-wasm for systems without esbuild.exe #1255

Closed
bridzius opened this issue Jan 5, 2022 · 9 comments · Fixed by #1276 or #1283
Closed

[Bug]: Doesn't fallback to esbuild-wasm for systems without esbuild.exe #1255

bridzius opened this issue Jan 5, 2022 · 9 comments · Fixed by #1276 or #1283
Labels
🐛 Bug Confirmed Bug is confirmed

Comments

@bridzius
Copy link

bridzius commented Jan 5, 2022

Version

11.0.0

Steps to reproduce

  1. Clone jest-preset-angular on a computer, which cannot launch the esbuild binary.
  2. Run the Angular 13 test stack.
    3. Transformation takes a long time/never finishes - tests are never run.

Expected behavior

Transformation should fallback to esbuild-wasm if esbuild binary is unavailable.

Actual behavior

Transformation tries to launch the esbuild binary, which is blocked and never starts the actual transformation. Transformation takes a long time/never finishes - tests are never run

Additional context

Many corporate environments are locked down, where only specifically signed binaries are allowed to run. The esbuild.exe binary on Windows is unsigned, so it can not be run on such systems.
This bug previously existed on @angular itself - angular/angular-cli#21687 - and was fixed via a similar fallback - angular/angular-cli#21763.
A similar approach could be used for jest-preset-angular. A quick test revealed that changing esbuild to esbuild-wasm in

import { transformSync } from 'esbuild';
does fix the problem on such locked down machines.

Environment

System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i5-10310U CPU @ 1.70GHz
  Binaries:
    Node: 14.17.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.17 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.15 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    jest: 27.4.5 => 27.4.5
@bridzius
Copy link
Author

Thanks for the fix @ahnpnl, but I've tested this (applying the changes manually to a local jest-preset-angular) and could it be that the fix doesn't really tackle the underlying issue?
The esbuild npm package is available on the system (downloaded). When esbuild is installed - a postinstall script actually downloads a esbuild.exe binary, which is located on the filesystem afterwards. On locked down machines - the esbuild package tries to run the esbuild.exe binary and the execution of that binary is blocked - that is what is causing the problem.
On Angular - this is solved by running a simple binary esbuild.exe command via https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/webpack/plugins/esbuild-executor.ts#L60 and https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/esbuild-check.js and seeing if that errors out (it errors/doesn't work if the binary can not be launched).
The current fix will work if the npm package is not present in the system, but in this case the package AND the binary are there, but the binary is unavailable (it cannot be executed), so the try block of the try/catch would still be successful.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Jan 17, 2022

Seem like we have to use spawnSync instead of require? Would you pls try that?

@bridzius
Copy link
Author

bridzius commented Jan 17, 2022

@ahnpnl thanks for looking into it!
A solution that basically replaces the current try/catch with something similar to the Angular solution based on spawnSync seems to work (though I've yet to check if this isn't just a false positive and still allows the running of the proper esbuild on systems that have it):

let useNativeEsbuild = false;
try {
  const esbuildCheckPath = require.resolve('@angular-devkit/build-angular/esbuild-check.js');
  const { status, error } = child_process.spawnSync(process.execPath, [esbuildCheckPath]);
  useNativeEsbuild = status === 0 && error === undefined;
} catch (e) {
  useNativeEsbuild = false;
}
this.esbuildImpl = useNativeEsbuild ? require('esbuild') : require('esbuild-wasm');
//<...>
//Inside of process()
const { code, map } = this.esbuildImpl.transformSync(fileContent, {
                loader: 'js', //...
//<...>

Unless you meant something else in your previous reply.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Jan 17, 2022

Thanks if that works I will make another attempt.

@bridzius
Copy link
Author

Thanks if that works I will make another attempt.

If you have a PR or a branch - I can try running it on the machine.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Jan 17, 2022

I created a draft PR #1283

@bridzius
Copy link
Author

I created a draft PR #1283

The draft looks pretty similar to the code that worked and that code DOES work. As long as that code correctly uses esbuild instead of esbuild-wasm on envs that have esbuild binaries available - that should work.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Jan 18, 2022

Thanks! I gonna merge it in main then 👍

@seth-broweleit
Copy link

I just updated all my angular packages from 12.2.3 to 12.2.13. When I ran test I got this

    > jest --config ./src/jest.config.js     
    Error: Cannot find module 'esbuild'
    Require stack:
    - C:\code\EDM_DIG_UI\node_modules\jest-preset-angular\build\ng-jest-transformer.js
    - C:\code\EDM_DIG_UI\node_modules\jest-preset-angular\build\index.js
    - C:\code\EDM_DIG_UI\node_modules\@jest\transform\node_modules\jest-util\build\requireOrImportModule.js
    - C:\code\EDM_DIG_UI\node_modules\@jest\transform\node_modules\jest-util\build\index.js
    - C:\code\EDM_DIG_UI\node_modules\@jest\transform\build\ScriptTransformer.js
    - C:\code\EDM_DIG_UI\node_modules\@jest\transform\build\index.js
    - C:\code\EDM_DIG_UI\node_modules\jest-runtime\build\index.js
    - C:\code\EDM_DIG_UI\node_modules\@jest\core\build\cli\index.js
    - C:\code\EDM_DIG_UI\node_modules\@jest\core\build\jest.js
    - C:\code\EDM_DIG_UI\node_modules\jest\node_modules\jest-cli\build\cli\index.js
    - C:\code\EDM_DIG_UI\node_modules\jest\node_modules\jest-cli\build\index.js
    - C:\code\EDM_DIG_UI\node_modules\jest\node_modules\jest-cli\bin\jest.js
    - C:\code\EDM_DIG_UI\node_modules\jest\bin\jest.js
        at Function.Module._resolveFilename (internal/modules/cjs/loader.js:902:15)
        at Function.Module._load (internal/modules/cjs/loader.js:746:27)
        at Module.require (internal/modules/cjs/loader.js:974:19)
        at require (internal/modules/cjs/helpers.js:92:18)
        at new NgJestTransformer (C:\code\EDM_DIG_UI\node_modules\jest-preset-angular\build\ng-jest-transformer.js:37:102)
        at Object.createTransformer (C:\code\EDM_DIG_UI\node_modules\jest-preset-angular\build\index.js:5:30)
        at C:\code\EDM_DIG_UI\node_modules\@jest\transform\build\ScriptTransformer.js:395:39
        at processTicksAndRejections (internal/process/task_queues.js:95:5)
        at async Promise.all (index 0)
        at async ScriptTransformer.loadTransformers (C:\code\EDM_DIG_UI\node_modules\@jest\transform\build\ScriptTransformer.js:379:5)

Then I updated jest-preset-angular to latest and it didn't fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Confirmed Bug is confirmed
Projects
None yet
3 participants