esm: add gotcha to documentation #32237

jonerer · 2020-03-12T20:37:36Z

Update es module documentation in accordance to #31710 by providing a gotcha for windows users

Checklist

documentation is changed or added
commit message follows commit guidelines

Update es module documentation in accordance to nodejs#31710 by providing a gotcha for windows users

doc/api/esm.md

Explains that absolute specifiers and absolute paths are not the same, and mentions what to do about it. Refs: nodejs#31710

jonerer · 2020-03-15T16:27:43Z

Thanks for the input! Now it's stuck on a js linting. Like this:

/home/runner/work/node/node/doc/api/esm.md
  806:1  error  Parsing error: Can not use keyword 'await' outside an async function

  3 | const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;
  4 | 
> 5 | await import(fileUrl);
    | ^
  6 |

✖ 1 problem (1 error, 0 warnings)

Makefile:1218: recipe for target 'lint-js' failed
make: *** [lint-js] Error 1
##[error]Process completed with exit code 2.

Should I just remove the "js" tag and be done with it?

himself65 · 2020-03-15T16:47:50Z

Thanks for the input! Now it's stuck on a js linting. Like this:

/home/runner/work/node/node/doc/api/esm.md
  806:1  error  Parsing error: Can not use keyword 'await' outside an async function

  3 | const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;
  4 | 
> 5 | await import(fileUrl);
    | ^
  6 |

✖ 1 problem (1 error, 0 warnings)

Makefile:1218: recipe for target 'lint-js' failed
make: *** [lint-js] Error 1
##[error]Process completed with exit code 2.

Should I just remove the "js" tag and be done with it?

Node.js not support top level await. So warp it to the async function

himself65 · 2020-03-15T16:49:58Z

doc/api/esm.md

+
+const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;
+
+await import(fileUrl);


example:

Suggested change

await import(fileUrl);

(async function() {

await import(fileUrl);

})();

I feel the code snipped should be short and to the point. An iife is a bit distracting. Maybe adding a ".then(...)" instead of await?

guybedford

Thanks for the docs PR @lulzmachine it's a huge help to clarify these things better in the docs.

guybedford · 2020-03-15T17:08:44Z

doc/api/esm.md

+
+const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;
+
+await import(fileUrl);


Maybe add a comment above noting what the value of fileUrl is:

// file:///c:/path/to/file.js

or similar, just so it is clear what is happening.

guybedford · 2020-03-15T17:09:23Z

doc/api/esm.md

@@ -801,6 +801,22 @@ property:

 ## Differences Between ES Modules and CommonJS

+### Absolute file paths don't work on Windows


We already have a section on module specifiers in the "Terminology" section above. I'd rather we move this note into that section along with the description of module specifiers, than as a specific difference between CJS and ESM.

Strictly speaking, yes it is a difference, but I think to try keep the docs organized we should keep sections together.

That makes sense. But this note does need to specifically mention Windows, for Ctrl+F compatibility. My idea was that platform differences belonged in a "gotchas" type section.

But I'm new to writing the node.js docs. Would you rather have it up by the "specifiers" section?

Ok I can get behind having this section be a specific compat issue section.

Can we change the title then to:

Suggested change

### Absolute file paths don't work on Windows

### File URLs not file paths

and move it to after the extensions section, since that should probably still come first.

Then perhaps with the description, we can get straight to the problem rather than having it in the title:

On Windows, importing file paths will throw since the drive letter is interpreted as a URL protocol. Instead a URL conversion is needed...

or something like that?

if you're importing a path, conversion is always needed!

I'm concerned that's a bit backwards; I would prefer to catch Windows' users eyes with the title, to make sure they understand it's an important section. And then go into the technical reasons about why it doesnt work for them in the body.

But again, I'm new here. Is your method more inline with how the docs are typically worded?

Ok. My primary concern was to bring the documentation in line with experience for Windows users (and for makers of libraries supporting Windows).

Another way to resolve this issue seems to be to change the implementation of ES Modules in node.js to enforce the specifiers not allowing a starting char of '/'. And to change this documentation PR to one that is general for both MacOS and Windows ("File URLs not file paths").

Thoughts?

it is valid for a relative url to start with / and i don't see any reason to limit that. In terms of changes to the runtime, adding a more detailed error message in ERR_UNSUPPORTED_ESM_URL_SCHEME when the scheme is a single letter might help.

Agree for the windows-specific bug, adding more context to the error message would be preferable. Then to keep the docs changes here in line with the suggestion above to provide the more general advice since this is not Windows-specific information (eg there are other character encoding issues that apply for all platforms).

That would be a really good way to highlight the error when it happens. Definitely a good change!

Regarding absolute file paths when running on MacOS. Can we put some code to detect if the user is trying to do absolute path imports, and warn them that this is unsupported? Like comparing the fileToUrl() output with "file://" + specifier to see if it's the same?

Right now there's the issue of library creators using import() in unsupported ways. For libraries that auto-import files, it's likely that they will implement loading on MacOS, find that absolute paths work (by luck) and ship that. Which causes errors for Windows users of those libraries.

This already happened with jest and babel. But I'm sure more lower-profile projects will follow, resulting in undetected non-platform-independent libraries on npm.

I think that there is an expectation that you can produce platform-independent code by sticking with the built in functions (except for the obvious ones like child_process and some fs functions). And I would like to explore what possibilities there are to keep it that way

The last subsection of the “Differences between ES Modules and CommonJS” section is “URL-based paths”. I think that the warning should go in there, either inside that section or in a new section after it.

All the other gotchas are platform-agnostic, and therefore more important than something Windows-specific (since they affect everyone, not just Windows users). I understand that Windows users are a majority of all users, but absolute paths are also not that common of a use case; when combining both a gotcha that doesn’t affect everyone with a feature that isn’t that commonly used (absolute paths), I think it should go near the end of the list.

himself65 · 2020-03-17T02:08:34Z

doc/api/esm.md

+```js
+import { pathToFileURL } from 'url';
+
+const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;


Suggested change

const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;

const fileUrl = pathToFileURL('C:\\path\\to\\file.js').href;

uppercase is necessary. We are using C: everywhere else in those docs.

related PR: #32294

GeoffreyBooth · 2020-04-01T03:45:20Z

doc/api/esm.md

@@ -801,6 +801,22 @@ property:

 ## Differences Between ES Modules and CommonJS

+### Absolute file paths don't work on Windows


The last subsection of the “Differences between ES Modules and CommonJS” section is “URL-based paths”. I think that the warning should go in there, either inside that section or in a new section after it.

All the other gotchas are platform-agnostic, and therefore more important than something Windows-specific (since they affect everyone, not just Windows users). I understand that Windows users are a majority of all users, but absolute paths are also not that common of a use case; when combining both a gotcha that doesn’t affect everyone with a feature that isn’t that commonly used (absolute paths), I think it should go near the end of the list.

GeoffreyBooth · 2020-04-01T03:45:40Z

doc/api/esm.md

+on linux and MacOS, and therefore they are also accepted.
+But on Windows, importing an _absolute path_ to a file will not work.
+To ensure cross platform compatibility, file paths should be converted into
+URLs.


As a matter of style, the Node docs avoid italics/unnecessary emphasis whenever possible.

github-actions · 2020-10-19T18:16:18Z

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 · 2020-11-19T00:02:48Z

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

Update esm.md

2bec61e

Update es module documentation in accordance to nodejs#31710 by providing a gotcha for windows users

nodejs-github-bot added doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation. labels Mar 12, 2020

jasnell reviewed Mar 12, 2020

View reviewed changes

doc/api/esm.md Outdated Show resolved Hide resolved

jasnell reviewed Mar 12, 2020

View reviewed changes

doc/api/esm.md Outdated Show resolved Hide resolved

ExE-Boss approved these changes Mar 15, 2020

View reviewed changes

ExE-Boss reviewed Mar 15, 2020

View reviewed changes

doc/api/esm.md Outdated Show resolved Hide resolved

esm: Update documentation with windows gotcha

ea679b7

Explains that absolute specifiers and absolute paths are not the same, and mentions what to do about it. Refs: nodejs#31710

jonerer force-pushed the patch-1 branch from 6f37d7a to ea679b7 Compare March 15, 2020 16:04

himself65 reviewed Mar 15, 2020

View reviewed changes

guybedford reviewed Mar 15, 2020

View reviewed changes

himself65 requested changes Mar 17, 2020

View reviewed changes

GeoffreyBooth reviewed Apr 1, 2020

View reviewed changes

SimenB mentioned this pull request Apr 22, 2020

Dubious statement regarding reserved specifiers nodejs/modules#505

Closed

BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72 Compare May 31, 2020 12:18

jasnell added the stalled Issues and PRs that are stalled. label Jul 3, 2020

aduh95 added stalled Issues and PRs that are stalled. and removed stalled Issues and PRs that are stalled. labels Oct 19, 2020

github-actions bot closed this Nov 19, 2020

Domiii mentioned this pull request Apr 13, 2022

ES Module loading with abolute path fails on windows #31710

Closed

mattseddon mentioned this pull request May 21, 2023

Postmortem: 0.8.12 => 0.8.17 failed to load on Windows iterative/vscode-dvc#3946

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

esm: add gotcha to documentation #32237

esm: add gotcha to documentation #32237

jonerer commented Mar 12, 2020 •

edited

Loading

jonerer commented Mar 15, 2020

himself65 commented Mar 15, 2020

himself65 Mar 15, 2020

jonerer Mar 15, 2020

himself65 Mar 17, 2020

guybedford left a comment

guybedford Mar 15, 2020

guybedford Mar 15, 2020

jonerer Mar 15, 2020

guybedford Mar 15, 2020 •

edited

Loading

devsnek Mar 15, 2020

jonerer Mar 15, 2020

jonerer Mar 15, 2020 •

edited

Loading

devsnek Mar 15, 2020

guybedford Mar 15, 2020

jonerer Mar 15, 2020 •

edited

Loading

GeoffreyBooth Apr 1, 2020

himself65 Mar 17, 2020

GeoffreyBooth Apr 1, 2020

GeoffreyBooth Apr 1, 2020

github-actions bot commented Oct 19, 2020

github-actions bot commented Nov 19, 2020


		const fileUrl = pathToFileURL('c:\\path\\to\\file.js').href;

		await import(fileUrl);

		@@ -801,6 +801,22 @@ property:

		## Differences Between ES Modules and CommonJS

		### Absolute file paths don't work on Windows

	### Absolute file paths don't work on Windows
	### File URLs not file paths

esm: add gotcha to documentation #32237

esm: add gotcha to documentation #32237

Conversation

jonerer commented Mar 12, 2020 • edited Loading

Checklist

jonerer commented Mar 15, 2020

himself65 commented Mar 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford Mar 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonerer Mar 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonerer Mar 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 19, 2020

github-actions bot commented Nov 19, 2020

jonerer commented Mar 12, 2020 •

edited

Loading

guybedford Mar 15, 2020 •

edited

Loading

jonerer Mar 15, 2020 •

edited

Loading

jonerer Mar 15, 2020 •

edited

Loading