Remove __$__nodeRequire usages #166686
Remove __$__nodeRequire usages #166686
Conversation
* remove most usages of require.__$__nodeRequire * stop using require.nodeRequire
| @@ -47,7 +41,7 @@ else if (typeof require?.__$__nodeRequire === 'function') { | |||
| // want to have it running out of sources so we | |||
| // read it from package.json only when we need it. | |||
| if (!product.version) { | |||
| const pkg = require.__$__nodeRequire(joinPath(rootPath, 'package.json').fsPath) as { version: string }; | |||
| const pkg = globalThis._VSCODE_PACKAGE_JSON as { version: string }; | |||
There was a problem hiding this comment.
This is lazy to avoid a require(package.json) when running because a built time we add version to the product.json. The only exception is running out of sources for why this is called here. Would be nice for startup perf to avoid the package.json lookup.
There was a problem hiding this comment.
In the ESM future there will be no more way to synchronously load anything. So, the only way forward will be something like _VSCODE_PRODUCT where the bootstrapper or embedder does these conditional loads. I suggest that we merge this with the vscode.context merging effort
| globalThis._VSCODE_NODE_MODULES = new Proxy(Object.create(null), { get: (_target, mod) => nodeRequire(String(mod)) }); | ||
|
|
||
| // VSCODE_GLOBALS: package/product.json | ||
| globalThis._VSCODE_PRODUCT_JSON = require('../product.json'); |
There was a problem hiding this comment.
Is there a reason product/package.json cannot go through the same proxy above?
There was a problem hiding this comment.
Yeah, _VSCODE_NODE_MODULES is debt and must eventually disappear whereas the _VSCODE_PRODUCT_JSON and _VSCODE_PACKAGE_JSON globals are likely staying (being merged with vscode.context...)
There was a problem hiding this comment.
I mentioned in chat before: when I worked on using esbuild to bundle CommonJS, I noticed that it would inline every require call it finds. We cannot inline product.json though (because some properties are added after the build) and that is why the IOptimizeCommonJSTaskOpts have an array of external to explicitly exclude modules from getting inlined. We just have to make sure that this change preserves it. The bundler for CommonJS is configured from here:
Line 343 in 8000435
I am confident. These are either dynamic (cannot be inlined) or are explicitly spelled out as require calls with the same path. There is also https://builds.code.visualstudio.com/builds/insider which I gave a quick test and I didn't see any errors. @bpasero what else needed to test this? |
helps with #160416