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

Holes in AMD API specification #26

Open
ghghost opened this issue Jan 15, 2015 · 5 comments
Open

Holes in AMD API specification #26

ghghost opened this issue Jan 15, 2015 · 5 comments

Comments

@ghghost
Copy link

ghghost commented Jan 15, 2015

I have an extensive background in writing software specifications (I wrote the gzip and zlib RFCs) and would like to help tighten up the AMD API documentation to a solid interoperable spec. Here are the issues I have noted.

  1. What value does 'define' return? Proposal: The 'undefined' value. (I do not consider it acceptable to leave this unspecified.)

  2. The doc says the module id should default to "the id of the module that the loader was requesting for the given response script." This introduces the concepts of "loader" and "response script" that are neither part of the JS specification nor of the AMD specification. JS code is normally loaded simply by virtue of appearing in a "script" element on a HTML page: the "loader" is built into the browser, and there is no module id involved, only a file name. Proposal: Start by clarifying what the doc means.

  3. In "module id format", the documentation says "Relative identifiers are resolved relative to the identifier of the module in which 'require' is written." Proposal: Clarify that this only applies to the 'require' callback passed to the factory function, and that the global 'require' does not allow relative identifiers.

  4. What is the value of a module (the value returned by 'require')? Proposal: If the factory is an object, or if the factory function returns a value that tests as true, it is that object/value; otherwise, it is the value of 'exports', even if 'exports' was not passed to the factory function (in which case it is an empty object). (The former is in the doc already, but the latter is not.)

There are also issues shared by 'define' and 'require' related to module ids:

  1. MAY or MUST 'define' and 'require' normalize the module name by (1) removing embedded '.' parts and (2) removing '..' parts where there is a preceding part? E.g., a/./b normalizes to a/b, a/b/../c/d normalizes to a/c/d, and a/../../d/e normalizes to ../d/e. Proposal: they MUST normalize.

  2. Are module names starting with '..' after normalization allowed? E.g., If module 'a/b' asks for '../../d', the normalized form is '../d'. Proposal: they are not allowed (MUST throw an error).

  3. MAY or MUST 'define' and 'require' throw an error if a module identifier does not have the proper syntax? Proposal: they MUST throw an error.

I may have missed some further details: these are the ones I encountered in the process of doing my own implementation of the API.

Regarding discussion, please note that I will not participate in any Google service such as Google Groups, given Google's stated goal of annihilating meaningful individual privacy and information autonomy.

@tbranyen
Copy link

Regarding 4, I'm a bit wary of mixed implementations. Exports/module.exports should be explicitly opted into. For instance:

define(["module"], function(module) {
  module.exports = "or that";
  return "this?";
});

In this case we have not opt'd into the simplified common js syntax, so I would expect that the module value would be "this?". Since falsy values are currently acceptable module return values, this gets even more complex. (Should they not be?) I think they should.

Then again anything that we can do to reduce this: https://gist.github.com/tbranyen/8480792 would be a positive impact to the specification.

@jrburke
Copy link
Contributor

jrburke commented Jan 23, 2015

In general, we have tried to stay light on some details unless implementers got caught up on them, or if it meant larger code that prevented small adapters for things like built/optimized files that did not need dynamic loading. For instance, if there are a bunch of MUSTs, they could increase the size of the implementation by adding checks that may not be needed after a build.

It is also hard to see committing a lot of time to tightening down this API as it seems to have worked out in practice, and ES6 modules are "just around the corner". Unfortunately, they have been "just around the corner" for a while, so hard to know how much that matters.

For what it is worth:

  1. I believe undefined has been assumed. No loader I know of actually makes use of a return value there.

  2. The browser does not understand module IDs by default, so not sure what to do there. There are three big pieces of a module system in my opinion, and while separate in some ways, they have interlocking pieces:

  • Declaring a module (what AMD primarily does).
  • loading modules, module loader.
  • Runtime, module-specific APIs (referred to as "module meta" in ES6 parlance). module.id, module.url, require.toUrl, require([]) are examples in AMD-ish systems.

A common link among them all are the module IDs, the strings used to reference modules. These are not actually paths (see loader plugin ID references for further proof), so to have a coherent system, it seems like specifying IDs and how declaring a module fits in with the other pieces of a module system could make sense.

  1. I think it is recommended if top level require() calls did not allow "../" IDs, as they would be outside the module ID space. I was a bit fuzzy on this in requirejs, so it supports those IDs in a limited fashion, but it can lead to unexpected results, and blurs the distinction between IDs and paths. For AMD loaders I make going forward, I would not accept those IDs for those reasons.

  2. Non-modular scripts loaded by an AMD loader usually don't reference any sort of export functionality, so I think in this case undefined makes sense as an allowed value.

  3. I think MUST makes sense just because the module loader could end up with two different entries in its cache, but also do not feel strongly about mandating this. If a loader does not do it, the loader likely does not work well, and will discover the need to do this on its own, or slowly fade away.

  4. My comments for 3) apply here. I recommend it going forward, but do not follow it currently in requirejs (any major versions in the future would).

  5. That gets into strictly defining what is allowed as an ID, probably also talking about relation to the other parts of a module system too.


In general, I do not feel urgency to address these things. Pull requests for more explanation text might help, but I am hesitant to stamp MUSTs.

Instead, I think it is better if we can use our time to help inform ES6 module system efforts, assuming you can get an actual dialog going with someone on the committee.

Or perhaps helping people making speculative es6-ish module tools today to get away from using paths for IDs. That will just cause problems later when it turns out those IDs should not be paths. I see why they do it since there is no module loader spec that would inform how to resolve IDs, but still uncomfortable to see code written like that.

@qfox
Copy link

qfox commented Jul 18, 2015

You should definitely look at https://github.com/ymaps/modules

Their API is not the best, but there are a lot of practically proved features that AMD should have. I really want AMD will have these features in standard.

@jrburke
Copy link
Contributor

jrburke commented Jul 20, 2015

The main new benefit in ymaps/modules seems to be allowing for asynchronously defined exports. This is moving away from the ES2015 module syntax, so I prefer to keep closer parity with ES2015. I expect that ES7+ await or similar feature will fill that feature intention in the language, so also prefer to just wait on that language change for that kind of feature.

@qfox
Copy link

qfox commented Jul 20, 2015

@jrburke Actually, it not so away. As I know await is like just a syntax sugar for resolving promises. But yeah, spec of ES7 should helps a lot. Okay, thanks for the answer ;-)

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

No branches or pull requests

4 participants