jorendorff / js-loaders

Pseudoimplementation of the proposed ES6 module loaders.
54 stars 7 forks source link

1.6.3.8 Loader.prototype.eval ( source ): Named eval please. #82

Open johnjbarton opened 10 years ago

johnjbarton commented 10 years ago

Historically eval() has been difficult to support in development tools because the source name is not defined. Typically (bad) choice is to assign the source to the name of the caller. This result in ambiguous mapping to file:line.

The historical hackaround is to postpend //# sourceURL and parse the name out of the source.

We have a chance to do better: allow a name to be passed to Loader.prototype.eval: Loader.prototype.eval( source, name ) Where 'name' has the same meaning as elsewhere (IMO this name should be passed to normalize() by the Loader). The 'name' will become the referer for any imports performed within source.

Another option is to follow 1.6.3.6 Loader.prototype.module ( source, options ): Loader.prototype.eval( source, options ) where options.address names the source. The Note in the module section becomes; NOTE: This is the dynamic equivalent of an script in HTML.

johnjbarton commented 10 years ago

OR: remove this function from the spec altogether.

Loader.prototype.module( source, options ) already does what I want: compiles JS with a url (options.address). Loader.prototype.eval( source ) is inferior and confusing.

jorendorff commented 10 years ago

Modules and scripts have different syntax (scripts eval synchronously, so they can't import) and semantics (a var in a module declares a variable that's local to that module, not a property on the global object), so we definitely want both— .eval() for scripts and .module() for modules.

You do probably want .module() rather than .eval() most of the time.

I think .eval() should support a second options argument as proposed.

johnjbarton commented 10 years ago

Consider naming the function .script() to clarify its role and relationship to .module() while avoiding confusion with many subtle issues of eval()

jorendorff commented 10 years ago

Yes we are going to propose dropping loader.eval() in favor of realm.eval(), but the issue still applies to realm.eval().

@dherman and I agree this is valuable and we want to tackle this in the ES6 timeframe. The API can be realm.eval(script, options) and we'll just specify that the spec does ToString(options.address) with a NOTE saying the spec doesn't use the result but the implementation may use it in error messages, debuggers, etc.

As a technical detail: the Loader specification must also do ToString(address) at some point, with a similar note. I don't think it does yet.

Separately from this, I think implementations do a bad job of just retaining the information that is already available (from the stack at the time eval is called). But that is a quality-of-implementation thing; perhaps there is not much TC39 can do about it.

johnjbarton commented 10 years ago

Just to point out that the stack at the time eval is called does not help us with the problem address by naming the buffer. The key issue with naming eval() and now module() buffers is stable identifiers across multiple executions so development tools can apply information-extraction operations. For examples, breakpoints and tracing queries need a way to know that a buffer means the same source on this run as the run that the developer used to specify the breakpoint or trace. Absent naming, the debugger has to fall back to using the source as the identifier, very expensive and brittle. The callstack is often identical across multiple buffers, for example in the case of loaders or transcoders, and thus cannot be used as the source of a stable name.

jorendorff commented 10 years ago

Just to point out that the stack at the time eval is called does not help us with the problem address by naming the buffer.

Completely agreed: it does not.