crypto: use map for lazy require cache

[dnalborczyk]

I'm curious why this lazyRequire function exists. wouldn't it be sufficient enough to call require directly instead and achieve the same 'lazy' result?

if that's the case, I can spin up another PR or repurpose this one.

[jasnell]

The lazyRequire() is named that way to make it clearer what is happening and that it's intentional. Switching to a map is good. Eventually, once we're sure all of the crypto stuff can safely be included in the startup snapshot (which is might be already, I'm not sure... @joyeecheung would know) then we can probably drop the lazy loading.

[joyeecheung]

@jasnell I can tell from the user land snapshot prototype that (at least a basic subset of) crypto works in user land snapshot, which doesn't assume the snapshotted state to be stateless since it'll re-initialize the command line arguments etc. during snapshot dehydration. However I am not yet sure if it's safe to include crypto in the startup snapshot which needs to be stateless, since it's a lot of code to vet for now..

[dnalborczyk]

thank you @jasnell !

I think what I was trying to say is that to my understanding webcrypto is being lazily loaded because of having a getter exported:

node/lib/crypto.js

Line 287 in 4f68839

get() { return lazyRequire('internal/crypto/webcrypto').crypto; }

webcrypto: {
  configurable: false,
  enumerable: true,
  get() { return lazyRequire('internal/crypto/webcrypto').crypto; }
},

lazyRequire uses a map for required references, but to my understanding that's what require() itself is already doing.

unless the internal require would be hoisted or the internal require works different than the "global" require, this would be pretty much the same:

webcrypto: {
  configurable: false,
  enumerable: true,
  get() { return require('internal/crypto/webcrypto').crypto; }
},

and for the crypto sub-modules loading, e.g. this:

node/lib/internal/crypto/webcrypto.js

Line 85 in 4441c3e

return lazyRequire('internal/crypto/rsa')

case 'RSA-OAEP':
      return lazyRequire('internal/crypto/rsa')
        .rsaKeyGenerate(algorithm, extractable, keyUsages);

would be pretty much the same as:

case 'RSA-OAEP':
      return require('internal/crypto/rsa')
        .rsaKeyGenerate(algorithm, extractable, keyUsages);

both of the above examples using require instead are still requiring the modules lazily. in fact, lazyRequire seems to be just adding additional complexity without any benefits.

[aduh95]

This needs a rebase.

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

[github-actions]

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.