Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test modules again #316

Open
fflorent opened this issue Nov 16, 2014 · 89 comments
Open

Test modules again #316

fflorent opened this issue Nov 16, 2014 · 89 comments

Comments

@fflorent
Copy link
Contributor

Issue #240 has removed the module tests from the table.
They should eventually be back, right? :)

Florent

@dead-claudia
Copy link

Yes, they should.

Actually, part of the problem was that, until recently, most of the module semantics itself was in flux. Traceur was changing parsing and some of its internals very frequently during that time, and no sane implementor (transpilers excluded) is going to implement a continuously changing part of any spec, no matter how progressive they are (Blink and SpiderMonkey tend to be eager to implement experimental features, but nobody has actually implemented it).

Now, it is relatively stable, and many more people are starting to prefer them to CommonJS's require() and AMD's define()/require(). Currently, though, given the SystemJS loader and the vast amount of the easier, still very interesting, things to implement, engine developers aren't likely to implement this quite yet (hopefully soon, though).

@webbedspace webbedspace added the ES6 label Dec 1, 2014
@webbedspace
Copy link
Collaborator

There are two large (but not insurmountable) problems with testing modules:

  1. Modules are async (same problem with testing Promise)
  2. Modules require a <script type="module"> tag - testing support for this will require a different approach than just running code in a normal <script> tag.

@dead-claudia
Copy link

Actually, it appears that the module loader itself isn't quite specced out yet, and much of the specification work has been stalled by needed work in other areas, especially the ECMAScript version 6 specification draft. I would suggest that it should be closed for now, until there is actually much of a specification to implement.

One thing I did notice is that the loader spec, even though it is hosted by WHATWG, is being written with Node.js in mind, potentially as an implementer itself. That opens up a lot of possibilities.

@kangax
Copy link
Collaborator

kangax commented Dec 29, 2014

@webbedspace according to http://www.2ality.com/2014/09/es6-modules-final.html we can't use eval

@fflorent
Copy link
Contributor Author

Sorry for the delay of the answer.

  1. Modules are async (same problem with testing Promise)

Oh! Why is that a problem?

@webbedspace according to http://www.2ality.com/2014/09/es6-modules-final.html we can't use eval

If I understand well, the idea of using eval is to avoid errors (and especially syntax errors) preventing the other tests to run, right?
So what about putting the test in an iframe? If I am not wrong, a JS error in an iframe doesn't stop the execution of the scripts in the parent windows?

Florent

@dead-claudia
Copy link

Testing a Promise shouldn't be nearly that big of a deal, nor should loading itself. The bigger stumbling block is that the loader spec is still in active development, and isn't even complete enough to implement pieces of yet. Parsing support is the most you could possibly test, and even that is, AFAICT, impossible to test in a browser environment because of the ES6 spec and existing, stable HTML5 spec.

Here's what is realistically required to test for module support in a web browser environment, with specification status below each one:

  1. An element dedicated for modules.

The most likely syntax I seem to be hearing at this point is simply using the traditional import syntax with a loader for importing other modules (i.e. no extra elements have to be added) and this for immediately loaded and executed modules:

<script type="module">
import foo from 'bar.js';
console.log(foo);
</script>
<script type="module" src="foo.js"></script>

The larger issue is that of "do HTML implementors implement this?", since it requires these semantics to be properly recognized and parsed.
2. A consistent loader API for loading and manipulating modules from regular <script> elements and dynamic loading of modules from other modules, as opposed to static module imports. This appears to be where the loader API stands at the moment, based on the ES6-turned-WHATWG loader specification. It targets both server-side and browser implementors. Many parts are just an educated guess based on the previous version in the ES6 specification.

// Dynamic import
System.import(name: string, options: object) -> Promise

// Load, but do not evaluate
System.load(name: string, options: object) -> Promise

// Alias an import to its source (I think...just an inference)
System.provide(name: string, src: string) -> undefined

// Hook to affect how paths are normalized
System.hook(
  "normalize",
  normalize: (name: string, referrer: string, address: string) -> string
) -> undefined

// Hook to affect how modules are located
System.hook(
  "locate",
  locate: (loadRequest: Object) -> Object
) -> undefined

// Hook to affect how modules are fetched
System.hook(
  "fetch",
  fetch: (loadRequest: Object) -> string
) -> undefined

// Hook to translate/compile source code after being fetched, but before
// fully loaded, such as compiling CoffeeScript files. Similar to Node's
// deprecated (but unremovable - the API is locked) require.extensions.
System.hook(
  "translate",
  translate: (loadRequest: Object) -> string
) -> undefined

// Hook to support importing pre-ES6 modules, such as CommonJS or AMD
System.hook(
  "instantiate",
  instantiate: (loadRequest: Object) -> Object
) -> undefined

// Respective getters for each
System.hook("normalize")
System.hook("locate")
System.hook("fetch")
System.hook("translate")
System.hook("instantiate")

// A (string, module) map serving as a cache, to avoid repeat loads.
// Similar to the require.cache object in Node.js
System.modules: Map

Note that the inner workings are largely under development currently, anyways. No implementation exists yet, although the previous version that was in the spec was already polyfilled at this point. The most popular one, es6-module-loader, has begun to stall a little after the loader was removed from the ES6 specification draft.
3. A way to dynamically load a module. In the browser, you can dynamically create <script type="module">, which should be easy to do with an enclosing <iframe>. That would test browser recognition of the syntax.

function testModule(src, expected, cb) {
  'use strict';

  var waiting = true;
  var failedParse = false;
  var answerFailed = false;
  var i = 0;
  var timer;
  var cache = {};

  function ret(str, reason) {
    if (!(reason in cache)) {
      cache[reason] = false;
      cb(str == null ? str : new Error(str), reason);
    }
  }

  var iframe = $('#test-iframe');
  var iframeWindow = iframe.attr('contentWindow');

  iframeWindow.resolve = function (val) {
    if (val !== expected) {
      ret('Expected: ' + expected + ', Found: ' + val, 'value');
    } else {
      ret(null, 'resolved');
    }
  };

  var script = $('<script>')
    .attr('type', 'module')
    .attr('text', src
      .replace('\x3c!--', '\\x3c!--')
      .replace('\x3cscript', '\\x3cscript'));

  iframeWindow.onerror = function (msg) {
    ret(msg, 'error');
  };

  iframe.empty().after(script);
}

In Node, it would be far simpler except for the required temp file and asynchronous event loop work surrounded by a try-catch to ensure it doesn't kill the program. It's otherwise as simple as child_process.fork(). It should be similar for other server-side JavaScript implementations.

function testModule(src, expected, cb) {
  'use strict';
  var fs = require('fs');

  var tmpfile = './tmp.js'; // or whatever works best
  var called = false;
  var cache = {};

  function Wrapper(err) {
    this.err = err;
  }

  function ret(err, reason) {
    if (called === false) {
      cache[reason] = called = true;
      fs.unlink(tmpfile, function (unlinkErr) {
        if (unlinkErr) {
          throw new Wrapper(unlinkErr);
        } else {
          cb(err, reason);
        }
      });
    } else if (!(reason in cache)) {
      cb(err, reason);
    }
  }

  fs.writeFileSync(tmpfile, 'utf8', src +
    ';function resolve(v){process.send(v)};');

  try {
    require('child_process').fork(tmpfile)
    .on('message', function (val) {
      if (val !== expected) {
        cb(new Error('Expected: ' + expected + ', Found: ' + val), 'value');
      } else {
        cb(null, 'resolved');
      }
    })
    .on('error', function (err) {
      cb(err, 'error');
    });
  } catch (e) {
    if (e instanceof Wrapper) { // if the unlink failed
      throw e.err;
    } else {
      cb(e, 'error');
    }
  }
}

These implementations are self-contained here, but can be edited to be less so. Make sure to call resolve(returnVal) to return the value to the parent if you use this, and the callback is called with two arguments: the error, if it exists, and the resolution reason, one of 'error' for an error in loading the script, 'value' for an incorrect value, and 'resolved' for a verifiably correct value.

I know this is quite a hack, and relies on some very platform-dependent behavior, but something like this should suffice to test modules, once the specification stabilizes. But, as it stands now, the loader itself is far too immature to reliably test much of anything.

@kangax
Copy link
Collaborator

kangax commented Jan 16, 2015

As of now, I don't see a good way to test for modules support at all — http://es-discourse.com/t/how-is-module-code-and-global-code-differentiated/ We could try <script[type=module] or <module> for browsers but even that is uncertain at the moment, from what I understand, and for non-browsers it's unclear even more. Feels like shooting in the dark.

@dead-claudia
Copy link

I agree. I would suggest closing this for now, and opening a new one once there's a stable enough standard to actually test against. Also, such tests would, by necessity, have to be somewhat platform-specific.

And WRT Node, there doesn't even exist any solution AFAIK outside of es6-module-loader and friends.

@kangax
Copy link
Collaborator

kangax commented Jan 28, 2015

V8 folks are already working on modules — https://code.google.com/p/v8/issues/detail?id=1569#c7 — so we'll have at least few implementations to test soon (Traceur, 6to5, V8)

@dead-claudia
Copy link

Module parsing is irrelevant without a full loader, though. The issue and
CL are a WIP for parsing support.
On Jan 28, 2015 3:52 PM, "Juriy Zaytsev" notifications@github.com wrote:

V8 folks are already working on modules —
https://code.google.com/p/v8/issues/detail?id=1569#c7 — so we'll have at
least few implementations to test soon (Traceur, 6to5, V8)


Reply to this email directly or view it on GitHub
#316 (comment).

@kangax
Copy link
Collaborator

kangax commented Jan 28, 2015

This is what they already have — https://chromium.googlesource.com/v8/v8/+/f7dc15febeea78b22de1f57c397a3221a43d9213/test/mjsunit/harmony/module-linking.js

Only I'm not sure what that module R { ... } syntax is about. I don't see anything like this in latest ES6 spec.

@dead-claudia
Copy link

That was the old spec. Such a module literal was pulled at least six months ago. What they already have is known to be outdated. The current spec is being both reconstructed from old specifications, modernized, and generally developed here: http://whatwg.github.io/loader/
It is planned to become a living standard, but it isn't even developed far enough for that.

@probins
Copy link

probins commented Feb 28, 2015

I'd suggest adding a note on the web page somewhere explaining why modules aren't in the table even though they're part of the ES6 spec

@paulirish
Copy link
Contributor

paulirish commented Feb 28, 2015 via email

@kangax
Copy link
Collaborator

kangax commented Feb 28, 2015

I noticed V8 folks actively working on it lately so it looks like we'll be having browser implementations pretty soon. So far it's only compilers, as far as I know (babel, traceur).

@dead-claudia
Copy link

I don't think they are going to do much on the module linking side, though.
On Feb 28, 2015 5:24 PM, "Juriy Zaytsev" notifications@github.com wrote:

I noticed V8 folks actively working on it lately so it looks like we'll be
having browser implementations pretty soon. So far it's only compilers, as
far as I know (babel, traceur).


Reply to this email directly or view it on GitHub
#316 (comment).

@probins
Copy link

probins commented Mar 1, 2015

the loader is a separate story. As it's no longer part of the ES spec, it doesn't really belong here, and ISTM it would be better on the caniuse site. Having said that, support for module parsing isn't much use without a loader, so the two are closely intertwined. A new version of the polyfill based on the new spec has just been published https://github.com/ModuleLoader/es6-module-loader/tree/1.0/src but I too doubt whether anyone will start work on any implementations until the spec is more advanced - at the moment, there are still a lot of todos.

@silverwind
Copy link

As for testing syntax support on the server, I found this simple method:

$ babel-node -e 'process.on("uncaughtException", function(){console.log("modules supported")}); import a from "b.js"'
modules supported

In case of no support, it errors:

$ iojs -e 'process.on("uncaughtException", function(){console.log("modules supported")}); import a from "b.js"'
[eval]:1
"uncaughtException", function(){console.log("modules supported")}); import a f
                                                                    ^^^^^^
SyntaxError: Unexpected reserved word

One might run that in a vm instance to possibly catch that SyntaxError.

@dead-claudia
Copy link

+1 for that idea (WRT Node/etc.)
On Apr 16, 2015 15:02, "silverwind" notifications@github.com wrote:

As for testing syntax support on the server, I found this simple method:

$ babel-node -e 'process.on("uncaughtException", function(){console.log("modules supported")}); import a from "b.js"'
modules supported

In case of no support, it errors:

$ iojs -e 'process.on("uncaughtException", function(){console.log("modules supported")}); import a from "b.js"'
[eval]:1"uncaughtException", function(){console.log("modules supported")}); import a f
^^^^^^
SyntaxError: Unexpected reserved word``


Reply to this email directly or view it on GitHub
#316 (comment).

@backspaces
Copy link

Hope this isn't too noob, but why not just test for the existence of window.System, and its few well-known functions. The compat-table site is clear:

Please note that some of these tests represent existence, not functionality or full conformance

Axel Rauschmayer, in http://www.2ality.com/2014/09/es6-modules-final.html, says in the comments:

You could check for System and System.module() (and possibly even call the latter).

@kangax
Copy link
Collaborator

kangax commented May 23, 2015

I think System was removed at some point (possibly with intention to move it to loader API but last time I checked it wasn't there — please correct me if I'm wrong)

Sent from my iPhone

On 23 May 2015, at 13:05, Owen Densmore notifications@github.com wrote:

Hope this isn't too noob, but why not just test for the existence of window.System, and its few well-known functions. The compat-table site is clear:

Please note that some of these tests represent existence, not functionality or full conformance
Axel Rauschmayer, in http://www.2ality.com/2014/09/es6-modules-final.html, says in the comments:

You could check for System and System.module() (and possibly even call the latter).

Reply to this email directly or view it on GitHub.

@silverwind
Copy link

Yeah, no System or Reflect.Loader available in latest Babel. Why is this so damn hard to test for 😢

@dead-claudia
Copy link

Because the Web loader needs a spec to be worth anything. Server side runtimes can come up with their own loader for now. And the module resolution and loading itself is almost completely implementation-defined as per the spec. The first engine to likely be using ES6 module syntax will likely be io.js, since V8 has gotten the farthest on its implementation of the current syntax (and it already has numerous unit tests for it). SpiderMonkey has parsing support for the old syntax, but the buggy let/const support is blocking progress on it.

(Aside: It's unsurprising that V8 is the farthest along, considering it's the backend for Node and io.js, among several other CLI runtimes, and it's frequently embedded in other applications as well.)

@dead-claudia
Copy link

And @kangax, yes, it was, and that's the reason. Their goal is to standardize a promise-based module API across the server and client. Thing is, the initial draft is still a WIP, and the work is largely currently overshadowed by the ES6 implementation work, along with a possible errata for a few spec bugs.

Thankfully, the ES7 proposals are relatively small comparatively, in that they can be easily polyfilled/transpiled or implemented, with the most complicated part being async generators. (The rest can simply be desugared or implemented in plain ES6 in engines, including async functions, decorators, static properties, and the on operator. Object.observe and its Array counterpart are impossible in pure JS, but extremely easy on the implementer's side.)

@domenic
Copy link
Contributor

domenic commented Oct 8, 2016

Even if browsers can't currently fetch a module, could they parse and execute modules loaded through some other module loader?

They could. However, that is not required for being compliant with ES2015. It is 100% compliant with ES2015 to never parse modules at all. All that means is that the section of the spec for the module parse goal is never initiated in your engine. That's totally fine! There's no spec requirement that the Module goal ever be used. The spec just contains rules that apply if the Module parse goal is reached.

In other words, writing dead code that is never triggered does not make your engine more or less ES2015-complaint. Spec compliance deals with observable behaviors only, not the spec's internal algorithms, and from the ES2015 spec, module support is simply not observable. That's why it's accurate to say that modules are not in any interesting sense part of ES2015.

@indolering
Copy link

Thanks @domenic!

@flying-sheep
Copy link

to elaborate: if you execute an import statement in firefox’ console or scratchpad, you’ll get

SyntaxError: import declarations may only appear at top level of a module

which (unless you parse the error message) is indistinguishable from an engine not supporting the import syntax.


so we could test for basic support by checking if the syntax error message differs from other messages (an engine without import syntax support would say e.g. SyntaxError: unexpected token “import” at 1:1)

@ljharb
Copy link
Member

ljharb commented Oct 9, 2016

@flying-sheep that would be testing something that's not in the spec.

@zloirock zloirock mentioned this issue Nov 22, 2016
@ghost
Copy link

ghost commented Mar 17, 2017

I for one would like to see a PR and there appears to be near-consensus that this should be included in the table.

Agreed. Not mentioning ES6 Modules in the table is almost as good as suggesting they don't exist.

@ljharb
Copy link
Member

ljharb commented Mar 17, 2017

They don't, yet. #316 (comment)

@gavinaiken
Copy link

But the "next" tab of the table includes all the TC39 proposals down to Stage 0, and the whatwg/loader collection of ideas is surely at least as far along as things at that stage, since it has been implemented in Edge and Safari and is under development in Firefox and Chrome? So I don't understand why you'd want to include those far-off language features but not modules?

@ljharb
Copy link
Member

ljharb commented Mar 17, 2017

@gavinaiken Modules aren't part of the language spec; only most of the building blocks for them are.

Per #316 (comment), a separate tab for Modules might make sense now that there begins to be engines shipping them; but it'd have to only test what was in the spec; using engine-specific (untested) means to get to the Module parsing goal.

@gavinaiken
Copy link

Is that not possible? Sounds like it would be worth it if it is.

@ljharb
Copy link
Member

ljharb commented Mar 17, 2017

It's possible, but wouldn't be particularly fruitful at this time; although it would lay useful building blocks for the future. A PR to add a Modules tab would be interesting to review - but nobody's submitted one in the 5 months since I suggested it ¯\_(ツ)_/¯

@domenic
Copy link
Contributor

domenic commented Mar 17, 2017

the whatwg/loader collection of ideas is surely at least as far along as things at that stage, since it has been implemented in Edge and Safari and is under development in Firefox and Chrome?

That's not correct: https://github.com/whatwg/loader#implementation-status

none of them have begun work on the ideas prototyped here, since they are not ready for implementations yet.

@gavinaiken
Copy link

@domenic thanks for the correction, I did not realise that the whatwg/loader spec (or collection of ideas, whatever) was different from the browser implementation of <script type="module"> loading. Are the browser guys just making it up as they go along? Is there a spec they are working to? Can that be tested?

Re @ljharb's comment above about a PR to add a Modules tab, maybe the reason no one has submitted one yet is that they don't really understand the fine distinctions you folks on the committees have spent so long hammering out, so wouldn't know how to test what is agreed spec and what is still under discussion. I know I don't understand them well enough to attempt it...

@gavinaiken
Copy link

ps not meant to be a critical comment. Just a plea for more information and explanation really. Maybe a follow-up to https://blog.whatwg.org/js-modules ?

@domenic
Copy link
Contributor

domenic commented Mar 17, 2017

The spec for the script element is at https://html.spec.whatwg.org/multipage/scripting.html#the-script-element (as mentioned in that blog post)

@ghost
Copy link

ghost commented Mar 17, 2017

To quote @paulirish earlier in the thread:

The onus is not on the compat-table project to wait for the standards process to finish on a feature before it's included. The visibility here will drive forward all aspects of shipping features. [....] For better or worse, the presence in compat-table really influences this.

Would it be acceptable to add a few failing tests to get the section back, or is this a Catch-22?

@ljharb
Copy link
Member

ljharb commented Mar 17, 2017

@jhabdas per many comments in the above thread, the section does not belong on the ES2015 page. #316 (comment) is the way forward.

@indolering
Copy link

@jhabdas per many comments in the above thread, the section does not belong on the ES2015 page. #316 (comment) is the way forward.

I'm in total agreement, support from Node is still at least a year away. It's clearly very complicated and might require changes to the spec.

Let's create a new modules tab and go from there.

@ghost
Copy link

ghost commented Mar 19, 2017

@indolering @ljharb TBH I have no idea what "creating a tab" means, nor am I particularly interested in finding out. My intentions here are to drive module spec forward by raising visibility and need. And it's for that reason I added a PR the import() tests yesterday. Thank you.

@indolering
Copy link

@jhabdas We need an entirely new tab to track modules.

@ghost
Copy link

ghost commented Mar 19, 2017

@jhabdas We need an entirely new tab to track modules.

Please see #316 (comment)

@ljharb
Copy link
Member

ljharb commented Mar 19, 2017

@jhabdas presence of modules in the compat table will, at best, persuade implementations to implement (something that is not necessary; they are all implementing it), but at worst, persuade them to rush the implementation.

If you are not willing to "find out" or help create a new tab, then I'm afraid there's nothing you can do here to drive anything forward. Thanks for your comments.

@ghost
Copy link

ghost commented Mar 19, 2017

@ljharb I'm sorry if the table is not structured properly at current. I tried to help by adding dynamic import tests and you closed the PR because you want a tab first when per spec there's benefit for both classic and module scripts. It's not my MO to put the carriage in front of the horse. I usually drive a car.

@ljharb
Copy link
Member

ljharb commented Mar 19, 2017

As explained, the spec for import and import() can't be tested without environment-specific methods - the parsing goal is irrelevant there. The table is structured properly, which is why module tests don't appear anywhere. I'm not sure I get your metaphor, but if you want module tests on this table, please either help create a tab, or else there's no point in adding further noise to this thread.

If the tone of this thread continues to be problematic, I will lock it.

@ghost ghost mentioned this issue Mar 19, 2017
@indolering
Copy link

indolering commented Mar 20, 2017

Here's a Modules Tab ticket, please bikeshed there!

If the tone of this thread continues to be problematic, I will lock it.

Do it, this is a good tombstone and I'm tired of it spamming up my notification feed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests