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

Remove msrCrypto in favor of native APIs #12050

Merged
merged 6 commits into from
Nov 21, 2022
Merged

Remove msrCrypto in favor of native APIs #12050

merged 6 commits into from
Nov 21, 2022

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Nov 16, 2022

msrCrypto is a polyfill for the native APIs which are well supported in browser and node currently. The module also recommends using native APIs when they are available.

Fixes #12023
Part of #11996

msrCrypto is a polyfill for the native APIs which are well supported in
browser and node currently. The module also recommends using native APIs
when they are available.

Fixes #12023
Part of #11996
@Tyriar Tyriar added this to the November 2022 milestone Nov 16, 2022
@Tyriar Tyriar self-assigned this Nov 16, 2022
rzhao271
rzhao271 previously approved these changes Nov 16, 2022
connor4312
connor4312 previously approved these changes Nov 16, 2022
@Tyriar Tyriar dismissed stale reviews from connor4312 and rzhao271 via 59c2725 November 16, 2022 22:18
@Tyriar Tyriar requested a review from DonJayamanne November 16, 2022 22:19
@Tyriar Tyriar marked this pull request as draft November 16, 2022 22:20
@Tyriar
Copy link
Member Author

Tyriar commented Nov 16, 2022

I see this in the Lint task, but it's still passing:

ERROR in node:crypto
Module build failed: UnhandledSchemeError: Reading from "node:crypto" is not handled by plugins (Unhandled scheme).
Webpack supports "data:" and "file:" URIs by default.
You may need an additional plugin to handle "node:" URIs.
    at /home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/webpack/lib/NormalModule.js:832:25
    at Hook.eval [as callAsync] (eval at create (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/tapable/lib/Hook.js:18:14)
    at Object.processResource (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/webpack/lib/NormalModule.js:829:8)
    at processResource (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/loader-runner/lib/LoaderRunner.js:220:11)
    at iteratePitchingLoaders (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/loader-runner/lib/LoaderRunner.js:171:10)
    at runLoaders (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/loader-runner/lib/LoaderRunner.js:397:2)
    at NormalModule._doBuild (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/webpack/lib/NormalModule.js:819:3)
    at NormalModule.build (/home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/webpack/lib/NormalModule.js:963:15)
    at /home/runner/work/vscode-jupyter/vscode-jupyter/node_modules/webpack/lib/Compilation.js:1371:12
 @ ./src/platform/common/crypto.ts 22:66-98
 @ ./src/platform/telemetry/helpers.ts 4:17-44
 @ ./src/platform/telemetry/telemetry.ts 5:18-38
 @ ./src/extension.web.ts 44:20-61

2022-11-16 22:24:01: webpack 5.71.0 compiled with 1 error in 85656 ms

@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2022

Codecov Report

Merging #12050 (bfb0ec2) into main (9dcc6ce) will increase coverage by 6%.
The diff coverage is 90%.

@@           Coverage Diff            @@
##            main   #12050     +/-   ##
========================================
+ Coverage     63%      70%     +6%     
========================================
  Files        497      495      -2     
  Lines      35701    30989   -4712     
  Branches    5468     4861    -607     
========================================
- Hits       22677    21759    -918     
+ Misses     11082     7384   -3698     
+ Partials    1942     1846     -96     
Impacted Files Coverage Δ
src/platform/common/crypto.ts 86% <90%> (-2%) ⬇️
src/kernels/installer/productInstaller.node.ts 74% <0%> (-5%) ⬇️
src/kernels/raw/session/rawJupyterSession.node.ts 78% <0%> (-3%) ⬇️
...c/platform/common/process/pythonDaemonPool.node.ts 78% <0%> (-2%) ⬇️
...otebooks/controllers/controllerPreferredService.ts 73% <0%> (-2%) ⬇️
src/notebooks/debugger/debuggerVariables.ts 69% <0%> (+<1%) ⬆️
src/kernels/kernelDependencyService.node.ts 82% <0%> (+<1%) ⬆️
src/kernels/helpers.ts 70% <0%> (+1%) ⬆️

@connor4312
Copy link
Member

connor4312 commented Nov 16, 2022

@Tyriar I suggest using the IgnorePlugin to ignore /^node:/. I'm surprised the whole build doesn't fail actually.

@Tyriar Tyriar marked this pull request as ready for review November 18, 2022 22:28
@Tyriar Tyriar requested review from rebornix and removed request for DonJayamanne November 18, 2022 22:28
@Tyriar Tyriar mentioned this pull request Nov 18, 2022
@Tyriar Tyriar merged commit ee8804a into main Nov 21, 2022
@Tyriar Tyriar deleted the tyriar/msrCrypto branch November 21, 2022 16:16
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.

Why do we use msrCrypto?
5 participants