docs: clarify dynamic imports and side effects

[aldeed]

The docs did not explain very well how and why to use dynamic import(). Now it does. This should be updated in 12 LTS and maybe other versions, too. Not sure how docs versioning works.

Checklist

• documentation is changed or added
• commit message follows commit guidelines

[Trott]

@nodejs/modules-active-members

[GeoffreyBooth]

Thanks for this PR. A few broad thoughts:

• import() is part of the language, and as a general rule we leave basic language documentation to others. Perhaps instead of including a mini-tutorial here, we could link to a resource like MDN that adds whatever detail you feel the Node docs are missing?

• Since ES modules generally shouldn't have side effects, it seems out of place to include such a detailed discussion of several ways to import modules for their side effects.

[aldeed]

@GeoffreyBooth

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import shows import() usage, but it doesn't mention about the need to destructure and rename module.default either. That was the confusion that led me to do this PR in the first place. If you think MDN docs should be improved instead, and then linked from here, that might be fine, too.

It does seem like large portions of this existing page could be linking to MDN instead, if that's the criteria. But I like seeing it in the Node docs because historically it didn't always match module spec behavior. I understand it's supposed to match the spec now, but it's still useful to explicitly say "for dynamic imports, we match the spec found here", etc.

Regarding side effects, they can and do have them. I had a need to do this with a module that I don't maintain, which is why I added it to these docs. (I agree it's not ideal to have side effects, but ideal is different from reality.) The MDN article I linked to even has a "Import a module for its side effects only" section with the same info.

So in summary, I'm ok with linking to MDN for whatever is there. If you have any more specific suggestions about what I should keep and what I should link, please let me know.

[devsnek]

I think we should just link to MDN

[GeoffreyBooth]

I think anything that’s not particular to Node should be left out of the Node docs. Side effects are a general language thing not particular to Node; we can just link to a section of the MDN page for them.

The MDN docs themselves are a wiki; you can edit those pages.

[MylesBorins]

Hey team, seems like with the time that has passed we don't want to land this. Do we want to add a note pointing to MDN?

[GeoffreyBooth]

I would be fine with an early mention of import() in the ESM and/or Module docs linking to MDN.

[jasnell]

Left some feedback comments

[aldeed]

I can add links to MDN and adjust the text for the "anything that’s not particular to Node should be left out of the Node docs" guideline before this is merged.

[aldeed]

I updated the dynamic imports section on MDN: https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Dynamic_Imports

And then removed some of that text from this PR and instead linked to MDN.

This can be merged now as far as I am concerned.

[guybedford]

This seems to have the approvals necessary to land, is there anything else holding it back?

[GeoffreyBooth]

This all seems fine, but there's not really anything particular to Node added in this PR. This feels more like content that should be in MDN, explaining basics about import() that apply to any environment. Indeed, it's already there, in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Import_a_module_for_its_side_effects_only and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Importing_defaults. Rather than repeating that text, could we just mention and link to it?

[aldeed]

@GeoffreyBooth What exactly do you want changed? I'm already linking to those MDN sections you mention, and the only things left in this PR are examples that I believe are unique to Node. If those examples (particularly about needing to destructure "default") are part of the spec, then I guess those could also be moved to MDN. But I don't know that they are.

I left the mention of importing CommonJS in here because the MDN article isn't about ESM-CJS interoperability and that I think is another thing unique to Node.

Also I left the import 'package-or-file' side effect example here because the MDN section doesn't mention importing a package entry point for side effects, and that seems like a Node-specific case.

[aldeed]

I guess to phrase one thing I said as a question, is it true that const { default: something } = is how you should get the default export when importing dynamically in all browsers / non-Node implementations? Or have some implemented it as const something =?

If they're all the same, then I'll move more of that to MDN docs. I do still think there's value in having the examples here that explicitly say "to import from CommonJS or ESM", which is a Node-specific clarification of the MDN docs. This has been the most confusing thing for people I've been trying to help transition to ESM.

I don't know who if anyone is "in charge" of MDN docs decisions, but another approach could be to remove some of these sections entirely from the Node docs and update the MDN docs to have Node-specific examples. I don't usually see Node examples called out in MDN docs, but I don't know if there's any rule against it.

[ljharb]

@aldeed yes, they're all the same - import() is part of the language, and there's nothing node-specific other than "how to resolve the specifier" (which is always engine-specific).

[GeoffreyBooth]

@GeoffreyBooth What exactly do you want changed?

I don't feel strongly, as this is useful information, but in general the Node docs should try to be specific to Node as much as possible and not include generic JavaScript guidance. The more superfluous information that our docs contain, the longer the docs get and the less people read of them 😄 and the more there is for us to ensure correctness of and to maintain.

To be honest, I don't see anything added in this PR that's specific to Node. The previous text already included the one Node-specific thing, that import() can be used in either CommonJS or ES module context. As far as I can tell, everything that this PR adds is either already covered in MDN or could be made more detailed there. So I guess if it were up to me I would just close the PR.

Is there anything in particular that you can point to that this PR adds that is specific to Node? Or that is an especially common hazard for users to fall into in a Node context, separate from other environments' ESM contexts?

[aldeed]

OK I've moved more of this over to MDN

What's left here is the addition of two links to those updated MDN sections, some minor clarifications of the language, and one anchor link fix. I rebased and squashed everything into one commit. These remaining changes seem worth merging to me, but feel free to close this if there isn't agreement.

Thanks all for helping me work through this.

[GeoffreyBooth]

Thank you for working through this with us!

[MylesBorins]

LGTM

[MylesBorins]

Landed in f92df33