js-loaders icon indicating copy to clipboard operation
js-loaders copied to clipboard

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

Open johnjbarton opened this issue 11 years ago • 6 comments

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 avatar Dec 17 '13 22:12 johnjbarton

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.

johnjbarton avatar Dec 18 '13 19:12 johnjbarton

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.

jorendorff avatar Jan 09 '14 16:01 jorendorff

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

johnjbarton avatar Jan 09 '14 17:01 johnjbarton

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.

jorendorff avatar Feb 04 '14 17:02 jorendorff

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.

johnjbarton avatar Feb 04 '14 18:02 johnjbarton

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.

jorendorff avatar Feb 04 '14 21:02 jorendorff