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

This doesn't entirely solve the url resolution relative to a module problem #75

Closed
Jamesernator opened this issue Nov 13, 2018 · 13 comments

Comments

@Jamesernator
Copy link

Jamesernator commented Nov 13, 2018

In the explainer it says that import: can be used as a solution to this issue however it really just moves the problem a step higher.

While simple examples like fetch('import:./blah.txt') or new Worker('import:./blah.js') would work we still have the issue of abstractions over those primitives e.g.:

// Tasklet.js
export default class Tasklet {
  constructor(workerUrl) {
    this._worker = new Worker(workerUrl)
    // ...
  }
  // ...
}
// someFile.js
import Tasklet from '../path/to/tasklet/Tasklet.js'

const tasklet = new TaskLet('import:./foo.js')

In this case any import: url we use with Tasklet will be resolved relative to the Tasklet.js file rather than someFile.js which isn't very useful, we'd still need a resolve-like function in this case to resolve it correctly (e.g. new Tasklet(import.meta.resolve('./foo.js')) or new Tasklet(import.meta.resolve('import:some-helper-lib')).

@domenic
Copy link
Collaborator

domenic commented Nov 14, 2018

Thanks for raising this! Your logic seems sound, which is bad news for import: URLs :-/.

Note that as currently envisioned you could get around this by forcing the parsing to happen in someFile.js, e.g. by doing new Tasklet(new URL('import:./foo.js').toString()). (The .toString() is optional, and will be done implicitly by the browser if you leave it out.) But this is not a great fix to force app authors to write. Maybe context-sensitive URLs are fundamentally in conflict with URLs being string-serializable, and the desire to treat their string serialization interchangeably.

Note that this problem is deeper than just the import:./x relative form. It also affects the import:foo form, when it comes to scopes. That is, if someFile.js and Tasklet.js have different meanings for the "foo" package, we have the same problem.

I'm not sure we can salvage this, although maybe myself or someone else will come up with a brilliant idea. At this point the only intermediate step that comes to mind, besides removing import: URLs entirely, is to retreat to having them only work on bare specifiers, and only on global-to-the-app mappings (i.e. those inside "imports", not nested into "scopes").

This still solves some of the very basic use cases, around e.g. <link rel="stylesheet" href="import:widget/styles.css">. But even then it's not great, as without scopes support, compositionality is broken, i.e. packages cannot do something like this.append(document.createElement('link')).href = 'import:subwidget/morestyles.css'

:(

@domenic
Copy link
Collaborator

domenic commented Nov 14, 2018

Cross-linking to original import: URL feature request at #23 , to see if we can salvage those use cases in a coherent way.

@domenic
Copy link
Collaborator

domenic commented Nov 14, 2018

Continuing my thinking out loud...

One of the main reasons the OP is troubling is that it means that built-in platform features (like new Worker()) are different from author-created abstractions or wrappers (like new Tasklet()). That is, import: URLs work as expected for platform features, but not for author-created abstractions.

This falls out of the current proto-spec's use of "active script". Inside the implementation of new Worker(), there is no "script" running, so the active script is the caller. Whereas inside the implementation of new Tasklet(), there is script running, so the active script is the callee.

At the cost of additional complexity, one could imagine trying to fix this by some mechanism to say "don't consider me a script for the purposes of import: URL resolution". I hesitate to even sketch what this would look like, though, as it seems extra-complex and fragile, plus very ad-hoc for this specific situation. Would we really start asking all library authors that accept URLs to update their script to get flagged this way? That seems terrible.

@ljharb
Copy link

ljharb commented Nov 14, 2018

What about an API to reify an object to represent the url, with context-sensitive data stored in an unobservable internal slot?

I suppose it would be the same as “new url”, ergonomically, but it might afford more flexibility than forcing full inline resolution?

@domenic
Copy link
Collaborator

domenic commented Nov 14, 2018

Yeah, I don't see that as being any better than new URL()...

@jkrems
Copy link
Contributor

jkrems commented Nov 14, 2018

As long as there is an explicit API like import[.meta].resolve('foo:css') or even just a mostly non-magical new URL('import:foo.css', import.meta.url), the JS side of using import URLs seems fairly okay. I understood import: URLs mostly as a way to support HTML or CSS APIs that had existing behavior for "foo.css" that couldn't be broken.

In JS it feels more confusing than useful to have call-site specific behavior outside of syntax. And yet-another syntax using the import keyword feels icky (import["foo.css"]? import of "foo.css"? private("foo.css") so people are happy that it's being used for something?).

@littledan
Copy link
Contributor

littledan commented Nov 14, 2018

To take a stab at the ergonomics issue: might

URL`import:foo`

possibly be nicer than new URL('import:foo')? We might want to eventually go this way anyway with trusted types (but I haven't written up this proposal variant yet; the current proposal does not use templates). You could imagine permitting "bare" import: URLs literally in HTML, and then requiring use of the URL constructor in other cases (I guess innerHTML becomes debatable, but other DOM manipulation would be easy to require a URL if it's the import scheme).

@eliasvasylenko
Copy link

I don't understand the defeatism about this, an extra new URL(...) or similar seems fine to me. It should be obvious enough to me that by default a relative URL will be resolved relative to, well, the location that it's resolved from.

That said, can we set out a clearer view of the solution space? We're talking about a situation where a user of an API is calling into a library, only to discover that the URL string they passed in is not getting resolved as they expected. There are broadly two ways to approach this:

Put the responsibility on the user to force early resolution, via e.g. new URL(...) or other suggestions here.

  • Pro: We don't have to wait for library authors.
  • Con: Extra work for user, possibly inconsistent user model between built-ins and libraries.
  • Con: Library authors may need to document this requirement anyway.

Put the responsibility on the library author by e.g. asking them to explicitly resolve relative to the caller.

  • Pro: User model remains consistent and simple.
  • Con: We have to wait for library authors to update their code.

But the choice isn't actually so stark, there's a gradual migration path. We can enable library authors to do the extra work to ensure their API is more convenient and consistent with built-ins, but we don't have to force them to, as users aren't exactly left stuck if library authors fail to adapt quickly. The user can always force resolution at the use-site if they need to. Given enough time we can expect the ecosystem to adapt enough that the user rarely has to worry.

As for how each approach might look in practice, there are a few obvious choices, most of which have already been mentioned.

User responsibility:

  • Force local resolution in code with explicit new Url(), URL template literal, utility function, etc.
  • Provide means in the import map to specify that a particular path should be "transparent" and defer to the caller.

Library responsibility:

  • Some way to flag a script wholesale as being transparent to relative imports.
  • Explicitly resolve against the caller where the URL is used, e.g. some utility to resolve against the nth script up the call stack.

Is this really so terrible?

@jkrems
Copy link
Contributor

jkrems commented Nov 14, 2018

Potentially relevant: https://github.com/sebmarkbage/ecmascript-asset-references contains a suggestion of import.resolve. Worth noting: It doesn't return a URL or URL string but a transparent reference.

@domenic
Copy link
Collaborator

domenic commented Nov 14, 2018

FYI whatwg/html#3871 is the version currently being considered for implementation. (import.meta.resolve() that returns a string.) We set that aside when we thought import: URLs might work, but since they don't (i.e., would require a function call like new URL() anyway), we'll probably go back to that.

@littledan
Copy link
Contributor

Is there a proposed solution for HTML and CSS? Maybe you could still permit import: URLs there (though there's still the choice of whether to resolve it against the active script or environment settings object).

@domenic
Copy link
Collaborator

domenic commented Nov 15, 2018

Right, my comment at #75 (comment) briefly talks about how we could salvage something for HTML and CSS.

@justinfagnani brought up the interesting point that we could maybe still use "the current file's URL" in those cases. By which I mean:

  • Store the URL that an external CSS file was fetched from, and use that for CSS url() functions
  • Use the document's URL (maybe not affected by <base>??) for HTML
  • Use the HTML module's URL for HTML modules

@justinfagnani
Copy link
Collaborator

justinfagnani commented Nov 19, 2018

Yeah, my original request for this feature was to be able to access cross- and internal-package resources from within CSS and HTML, where presumably the unresolved import: URLs are not forwarded in ways that lose the base URL of the document they originated from.

I don't think it's necessary in JS. Internal-package URLs can be created with new URL and import.meta.url. Cross-package URLs could indeed benefit from import.meta.resolve().

Code that's consuming URLs from markup/CSS might need to resolve import: URLs - eg, a custom element that's reading a src attribute. Seems like import.meta.resolve() might need to take a baseURL when resolving import: URLs, and such code can resolve immediately when reading URLs from documents.

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

7 participants