-
Notifications
You must be signed in to change notification settings - Fork 66
Conversation
002-es-modules.md
Outdated
// esm ~= Promise => { | ||
// default => result_from_evaluating_foo; | ||
// } | ||
es_namespace.then( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esm.then
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
What about transitivity between the two? |
@unional it will not be possible unfortunately. |
cc/ @blakeembrey |
002-es-modules.md
Outdated
|
||
```js | ||
// use.mjs | ||
import {__dirname} from './expose.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't pluck __dirname
from the default
export :D
module.exports = __dirname;
import __dirname from './expose.js'
What about the case where one can EDIT: nm. This looks to be covered with the |
002-es-modules.md
Outdated
|
||
### 3.1. Determining if source is an ES Module | ||
|
||
A new file type will be used for ES Modules, `.mjs`. Files with this extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Files with this extension" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@trevnorris ah, need to make explicit what looks up things in core... are we trying to preserve being able to load non-core using that trick? several rules are already changing here, possible to block people doing such. |
@bmeck I'm not sure if doing |
All this use of ESM for modules makes me think if there is a new filename extension, shouldn't it be |
@thetalecrafter ES is the moniker for the specification of JS, but people use JS in conversation when not talking about the specification itself. Keeping the file extension similar to .js with the JS moniker was taken into account while making the decision on file extensions. Some considerations were listed in previous versions of this EP: https://github.com/nodejs/node-eps/blob/5dae5a537c2d56fbaf23aaf2ae9da15e74474021/002-es6-modules.md#511-reason-for-decision |
@bmeck With https://github.com/bmeck/UnambiguousJavaScriptGrammar being accepted isn't the file-ending part obsolete? The problem now however is not that we can't identify what type a file is. It becomes a Ecosystem problem and I am not sure that is something Node/Core should be fixing. |
@martinheidegger that was merged into the draft of the ESM proposal. The problem of determining [source text] type remains, and as stated in the current text of the proposal lack of acceptance by TC39 is in play here. The standard way to determine the contents of a file on any file system is to use a file extension. I have very little hope that my agenda item on TC39 will make any progress and am moving to what I see as the more realistic option. |
I would also like to note existing talking points about differing from browsers being bad, accidental mode swaps (in part related to differing behavior) being bad, and security implications if there exists ambiguity. |
Would you be open to a flag that enables "everything is an ESM mode"? |
@matthewp from my understanding currently there are issues with the spec that would mean for example: native code doesn't work at all with es6 modules, many core modules can't work at all with es6 mode. I may be way off here, but I think those are some of the reasons why that could not happen. |
Ah, I wasn't thinking about core modules, ok then. :( |
@bmeck I find it extremely sad to believe that. I have more hope tbh. Aside from that: If Node.js just assumes your proposal as accepted it might increase the chance for it to actually be accepted. |
@martinheidegger Node can't move it to accepted unless the intent is to implement it that way, Node can implement the "spec extension" of requiring an import as noted in various places (https://twitter.com/awbjs/status/744700272190971904 , #33 (comment)). However, this has implications that are problematic and Node very much would not like to have, including the ones listed above. If accepted I highly doubt the stance would change given there are already people stating the lack of need to change things. |
I want to publically state that there is a need to change things. It is an embarrassingly simple solution that would fix a major problem by creating a little effort for a minority of users and makes everyone's life better. Worth to champion for. (I am not sure I said this enough: thanks for your efforts) Edit: Also posted it on es-discuss |
*sigh* rebased ontop of #39
002-es-modules.md
Outdated
|
||
There is knowledge of breakage for code that upgrades inner package | ||
dependencies such as `require('foo/bar.js')`. As `bar.js` may move to | ||
`bar.mjs`. Since `bar.js` is not the listed entry point this was considered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, a common community "best practice" (although not a universal one) is to always omit extensions when requiring - to allow for "bar.js" to become "bar/index.js" transparently, for example - and require('foo/bar') would not break under this proposal.
@bmeck Will something like
? Is |
@michael-ciniawsky any solution that encapsulates use cases and is not prohibitive to adoption or education is open for discussion. Defense of .js is a very well thought out solution, but remains complex in order to achieve high use case coverage. As per [edit] note that a goal is hopefully to allow a path that all new code can be written in ESM without causing a perpetual burden to either Node or Browsers. |
👍 What parts of Defense for '.js' are prohibitive ?
? |
As has been stated many times, the decisions here are about weighing cost/advantage of all the things. There are also downsides to |
IMO as things moving forward, this scenario would be rare. 🌷 |
That shouldn't be the case of course,
kk, is separation not even better ? Why in
$HOME/.noderc
|
There were discussions of using
Would be adding a file in a manner that we haven't seen before. Unsure how core would feel about making a settings file standard. You could bring it up though, might be useful even w/o ESM.
This is complicated, it depends on what you setup in
On package.json: we add 3 new fields in package.json (which are interdependent), potentially a settings file, and still gotchas about figuring out if a .js is supposed to be one mode or the other. On file extension: we add a character to your file, and make sure to update code editors etc. to know about the MIME. On pragma / grammar change: we are similar to file extension (w/o MIME question), but won't get it through standards. Questions remain about ambiguity if not mandatory. |
As a module consumer (I've never written any npm packages), I would rather be aware which module format I'm using than be bitten by unexpected bugs. For example, webpack supports AMD module format and it has already caused issues for me (leaking global). If I'm maintaining an old codebase ( If I'm mainaining a new codebase, I would rather expect new version of an npm package to contain es6 version of a module rather than having to append Would it be reasonable to expect a The above is from a consumer perspective. Producers would probably want their packages to work indefinitely without any maintenance. |
…upport, timing, etc.
web UI problem caused me to squash, old branch is at https://github.com/bmeck/node-eps/tree/rewrite-esm-bak @targos please re-review |
002-es-modules.md
Outdated
|
||
1. Apply the URL parser to `specifier`. If the result is not failure, return the result. | ||
2. If `specifier` does start with the character U+002F SOLIDUS (`/`), the two-character sequence U+002E FULL STOP, U+002F SOLIDUS (`./`), or the three-character sequence U+002E FULL STOP, U+002E FULL STOP, U+002F SOLIDUS (`../`) | ||
1. Let `specifier` be the result of applying the URL parser to `specifier` with importing location's URL as the base URL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
github's markdown renderer is confused by 2-space indent... this renders as 3.
instead of an indented 1.
😅
maybe 4 spaces makes it happier?
002-es-modules.md
Outdated
1. Let `dir` be a new URL from `specifier`. | ||
2. If `dir` does not have a trailing `/` in its pathname append one. | ||
3. Let `searchable` be the result of applying the URL parser to `./package.json` with `dir` as the base URL. | ||
5. If the resource for `searchable` exists and it contains a "main" field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing step 4
(renumbered?)
002-es-modules.md
Outdated
5. If the resource for `searchable` exists and it contains a "main" field. | ||
1. Let `main` be the result of applying the URL parser to the `main` field with `dir` as the base URL. | ||
2. If it does not throw an error, return the result of applying the package main search algorithm to `main`. | ||
6. If it does not throw an error, return the result of applying the index main search algorithm to `dir`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The section this references is named just "Index search"
002-es-modules.md
Outdated
4. If it does not throw an error, return the result of applying the file search algorithm to `searchable`. | ||
5. If it does not throw an error, return the result of applying the directory search algorithm to `searchable`. | ||
6. Let `parent` be the result of applying the URL parser to `../` with `package` as the base URL. | ||
7. If it does not throw an error, return the result of applying the module search algorithm to `specifier` with an importing location of `parent`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/../
is still a valid path (..
in /
just points to itself)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/../
in URL parser becomes /
in step 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still won't throw an error, though, so this looks like it could loop forever (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would fail in step 1 when it tries to grab the directory of parent
which is /
. Can clarify in step 1 by saying if the parent is same as importing location it should error.
When a `package.json` main is encountered, file extension searches are used to | ||
provide a means to ship both ESM and CJS variants of packages. If we have two | ||
entry points `index.mjs` and `index.js` setting `"main":"./index"` in | ||
`package.json` will make Node pick up either, depending on what is supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it that there is no intent to support the ad-hoc module
field.
(it's better that way, tbh; module
is "specified" to require conversion to ES5 for example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no plan
1. if there was an error, place the error in the ESM cache and return | ||
2. let `export` be the value of `module.exports` | ||
3. if there was an error, place the error in the ESM cache and return | ||
4. create an ESM with `{default:module.exports}` as its namespace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this be a bound export, or will it snapshot the value of module.exports
at caching time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snapshot right then and there, grabs the value just like step 4. does not make a getter or anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it's as if it was wrapped with a module that did export default require('CJS')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roughly
002-es-modules.md
Outdated
|
||
These have `[[Set]]` be a no-op and are read only views of the exports of an ES | ||
module. Attempting to reassign any named export will not work, but assigning to | ||
the properties of the exports follows normal rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also not possible to add any extra keys
002-es-modules.md
Outdated
|
||
### 3.2. Determining if source is an ES Module | ||
|
||
A new file type will be recognised, `.mjs`, for ES modules. This file type will be registered with IANA as an official file type, see TC39 issue. There are no known issues with browsers since they [do not determine MIME type using file extensions](https://mimesniff.spec.whatwg.org/#interpreting-the-resource-metadata). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link for the TC39 issue?
What about direct execution of ESM code from the CLI? Should we specify this here? |
@targos I've left the CLI flags out of this EP as it is not something I think belongs in the EP itself. The general consensus is that you must include a |
landed in 6cc060e |
some discussions and problems with supporting various behaviors on a VM level, races with browsers, and upcoming spec changes have led to a drastic change in direction for the interop bridge.
https://gist.github.com/bmeck/52ee45e7c34d1eac44ce8c5fe436d753 has some relevant notes
notable:
import
is url based../
,./
, or/
use as base URL against current script and searchnode_modules
../
escapingnode_modules
import
ALWAYS unwind stack prior to any evaluationimport('old.js')
(spec being written) would always unwind the stack prior to evaluating.Promise<ModuleNamespace>
always, never synchronousdefault
export__filename
,__dirname
,require
,module
,exports
,arguments
this
value