Implements snapshot support #5741
Conversation
|
It seems that |
merceyz
left a comment
merceyz
left a comment
There was a problem hiding this comment.
It's supposed to make startup faster
If it doesn't then we shouldn't introduce extra complexity to support it.
| /* @internal */ | ||
| export function createAlgoliaClient(configuration: Configuration) { | ||
| const requester: AlgoliaTypes.Requester = { | ||
| async send(req: AlgoliaTypes.Request): Promise<AlgoliaTypes.Response> { | ||
| try { | ||
| const res = await request(req.url, req.data || null, { | ||
| configuration, | ||
| headers: req.headers, | ||
| }); | ||
|
|
||
| return { | ||
| content: res.body, | ||
| isTimedOut: false, | ||
| status: res.statusCode, | ||
| }; | ||
| } catch (error) { | ||
| return { | ||
| content: error.response.body, | ||
| isTimedOut: false, | ||
| status: error.response.statusCode, | ||
| }; | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| // Note that the appId and appKey are specific to Yarn's plugin-typescript - please | ||
| // don't use them anywhere else without asking Algolia's permission | ||
| const ALGOLIA_API_KEY = `e8e1bd300d860104bb8c58453ffa1eb4`; | ||
| const ALGOLIA_APP_ID = `OFCNCOG2CU`; | ||
|
|
||
| const algoliasearch = require(`algoliasearch`) as typeof import('algoliasearch').default; | ||
| return algoliasearch(ALGOLIA_APP_ID, ALGOLIA_API_KEY, {requester}); | ||
| } |
There was a problem hiding this comment.
This doesn't seem like something that belongs in the core.
There was a problem hiding this comment.
It's reused across a couple of plugins (two). The function is marked internal, so it's not part of the types / public API.
It makes the startup faster. It's not possible to enable it by default yet since it's still experimental in Node, but the Node folks need practical feedback to know when it's stable enough, and integrating it with Corepack (and Yarn) is a good way for them to get just that - while also giving us more options to make the startup faster in the future, outside of what we could do in userland. The extra complexity needs to be balanced, but it doesn't seem that bad to me overall. |
What's the problem this PR addresses?
Node.js supports snapshotting as an experimental feature. It's supposed to make startup faster. The drawback is that it doesn't work for various builtin modules, which need to be lazy loaded; in our case, this is:
How did you fix it?
Moved the aforementioned modules into lazy evaluation. The tty module is used at startup (by both chalk & clipanion), so I had to mock it.
My plan is to implement an experimental command in Corepack to build the snapshot for a given package manager, but that requires Yarn to first support it.
Checklist