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

Allow baseURL to be itself page-relative

Open guybedford opened this issue 10 years ago • 24 comments

It can be useful to write:

  System.baseURL = "../js"

At the moment, it is not clear if this would be supported by $ToAbsoluteURL.

This is from - https://github.com/ModuleLoader/es6-module-loader/issues/129

guybedford avatar Apr 09 '14 18:04 guybedford

To support this, would effectively be to do in locate:

  toAbsoluteURL(toAbsoluteURL(document.baseURI, this.baseURL), address + '.js')

guybedford avatar Apr 09 '14 18:04 guybedford

Actually, because toAbsoluteURL is being run in both locate and fetch, I would describe the logic as:

  • Locate: return resolve(address, this.baseURL) + '.js'
  • Fetch: return toAbsoluteURL(address, document.baseURI)

This way the fetch address is a normal URL, as expected like any other in the document, and we allow the baseURL to be a non-absolute URL.

guybedford avatar Apr 09 '14 20:04 guybedford

hmm, this sounds pretty weird. These rules seem very random to me.

caridy avatar Apr 09 '14 20:04 caridy

@caridy can you explain what you'd expect, and how that differs from this implementation or the suggested polyfill implementation? It would be good to come up with a solid set of scenarios for this.

guybedford avatar Apr 09 '14 20:04 guybedford

seems like customizing baseURL (which probably defaults to document.baseURI) is in the user-land, and it is responsibility of the user to define a custom value that its friendly to $ToAbsoluteURL() method.

caridy avatar Apr 09 '14 20:04 caridy

It seems odd to me that a URL returned by locate, still gets normalized relative to the baseURL. The fetch function should think of URLs as having the same meaning as other URLs on the page, so that is very much the justification behind the Fetch suggestion.

For Locate, this is entirely to support baseURL being a non-absolute URL. It certainly can be argued that baseURL should always be an absolute URL, but I think it will be a lot more flexible to users if it can be any relative URL. It seems an arbitrary distinction to allow a relative URL, but without backtracking.

It's great to discuss these, thanks for feedback.

guybedford avatar Apr 09 '14 20:04 guybedford

I want to change traceur's normalize() to prevent '../' from appearing in a normalized name. The only way to accomplish this is to take path segments from baseURL until the leading '../' segments are removed. A baseURL with '../' will cause this to fail.

The problem with '../' prefixes is that they prevent normalized names from being unique to modules. Since normalized names are one-to-one with modules, we can only name them one way. A leading '../' is ambiguous with a name where that segment is replaced with the value used to load the module. The value used to load the module is the only unambiguous name for that segment. Therefore we cannot have '../' leading segments. Since the only way to normalize() leading '../' segments requires at least as many path segments in baseURL as we have leading '../' segments, we should prohibit '../' in baseURL.

I also do not think that baseURL should be writable, it should be fixed by the loader at construction. Again this is because normalized names need to be one-to-one with modules. Rather than changing baseURL we should be changing the root dependency name.

I think that the key bit we have been missing in working with normalize() is the critical role of the name of the root module of a dependency tree. Within the dependency tree, all of the declarative modules are recursively relative to this root. When we name this root it has to normalize to a unique name, either beginning with './' or absolute. Otherwise we can't refer to this root any where else and be confident that we are talking about the same module.

(Obviously I wrote this much because I am trying to solve some problems which of course could have solutions I am not aware of).

johnjbarton avatar Apr 13 '14 15:04 johnjbarton

I do agree that a baseURL does not make sense to start with "../". As you say, they are absolute references by nature.

Yes, it makes no sense to change the baseURL after the initial load. But I expect that loaders would be frozen anyway by the user after having their baseURL and hooks set for security.

To clarify, are you generating normalized names that begin with "./"? Because I don't think this should be allowed either.

On 13 April 2014 08:50, johnjbarton [email protected] wrote:

I want to change traceur's normalize() to prevent '../' from appearing in a normalized name. The only way to accomplish this is to take path segments from baseURL until the leading '../' segments are removed. A baseURL with '../' will cause this to fail.

The problem with '../' prefixes is that they prevent normalized names from being unique to modules. Since normalized names are one-to-one with modules, we can only name them one way. A leading '../' is ambiguous with a name where that segment is replaced with the value used to load the module. The value used to load the module is the only unambiguous name for that segment. Therefore we cannot have '../' leading segments. Since the only way to normalize() leading '../' segments requires at least as many path segments in baseURL as we have leading '../' segments, we should prohibit '../' in baseURL.

I also do not think that baseURL should be writable, it should be fixed by the loader at construction. Again this is because normalized names need to be one-to-one with modules. Rather than changing baseURL we should be changing the root dependency name.

I think that the key bit we have been missing in working with normalize()is the critical role of the name of the root module of a dependency tree. Within the dependency tree, all of the declarative modules are recursively relative to this root. When we name this root it has to normalize to a unique name, either beginning with './' or absolute. Otherwise we can't refer to this root any where else and be confident that we are talking about the same module.

(Obviously I wrote this much because I am trying to solve some problems which of course could have solutions I am not aware of).

Reply to this email directly or view it on GitHubhttps://github.com/jorendorff/js-loaders/issues/105#issuecomment-40310863 .

guybedford avatar Apr 15 '14 18:04 guybedford

I'm unsure what you mean by "generating normalized names".

Ultimately we resolve the normalized name against baseURL in locate(). So the normalize() is effectively relative to baseURL. Since it needs to be unique, just about the only option seems to be normalized names that either begin './" (and resolve on baseURL) or are absolute, in particular, names that are package based (eg [email protected]/src/options).

If I follow this line of reasoning, normalized names need to only one of these two types, not starting '../'.

Does this make sense?

johnjbarton avatar Apr 15 '14 23:04 johnjbarton

@johnjbarton normalize should never result in a name that starts with ./ though.

Names like [email protected]/src/options always resolve to the baseURL, becoming baseURL/[email protected]/src/options.

So if we allow ./app and app, they both resolve to the same thing and we've lost the uniqueness of the normalized name.

Note that the normalize function in this repo will not allow this form either, as it always removes . or .. segments in the normalized name - https://github.com/jorendorff/js-loaders/blob/master/browser-loader.js#L108.

guybedford avatar Apr 16 '14 16:04 guybedford

@guybedford now I am really confused.

I though the idea of [email protected]/src/options was to allow System.map to return an address that the dev configured.

I don't propose both ./app and app as the same thing, on the contrary they are unique. ./app means "relative to baseURL"; app means "app".

Please see https://github.com/jorendorff/js-loaders/blob/master/browser-loader.js#L107:

// "." and ".." are allowed only at the start of a relative name.

My dot is only at the start and it means relative name.

johnjbarton avatar Apr 16 '14 17:04 johnjbarton

@johnjbarton ok sorry I thought you meant a ./ at the beginning of a normalized module name. It is important to distinguish carefully these two! ./ can't be seen in normalized module names though.

guybedford avatar Apr 16 '14 17:04 guybedford

I'd like to understand why we can't use ./ in normalized names.

If ./src/options mean 'relative to referrer' when used in an import directive, then why can't it mean 'relative to baseURL' when used in a normalized name?

If all of the normalized names are relative to baseURL, then the ./ seems harmlessly redundant as as long as we are consistent.

johnjbarton avatar Apr 17 '14 00:04 johnjbarton

All normalized names are located relative to baseURL by default.

src/options locates to baseURL/src/options.

So if we allow ./src/options, we've lost uniqueness.

guybedford avatar Apr 17 '14 01:04 guybedford

If all of the names are normalized to ./ then they will be unique. I think with vs without ./ is arbitrary as long as normalize is consistent.

johnjbarton avatar Apr 18 '14 14:04 johnjbarton

We would need to globally agree to use ./ for all normalized names.

The default System normalization doesn't do this currently.

Bear in mind this also affects things like System.get - System.get('./jquery').

guybedford avatar Apr 18 '14 17:04 guybedford

I agree that we have to define System.normalize() in the standard.

The current default System.normalize() just returns it's first argument.

System.get() takes a normalized name, so passing ./jquery' differs from passingjquery`. Which answer do you want?

I prefer the ./ in normalized names because normalized names are relative to baseURL and when un-normalized names are relative they start with ./. Absent a referrer (as is the default with System.import and System.module), an unnormalized name like ./jquery would normalize to ./jquery which I think makes the loader easier to understand. It is my experience with these dynamic forms mixed between browser and node that has lead me to raise this issue.

johnjbarton avatar Apr 18 '14 17:04 johnjbarton

System.normalize will be defined by the browser spec. And there is already an implementation right here. I'm trying to follow this implementation as closely as possible, and currently it will omit leading ./ from the normalized name regardless. So normalized names will never start with ./ according to this implementation (https://github.com/jorendorff/js-loaders/blob/master/browser-loader.js#L91, https://github.com/jorendorff/js-loaders/blob/master/browser-loader.js#L125).

It is something we do have to agree on though.

guybedford avatar Apr 19 '14 17:04 guybedford

My understanding, and how it works in AMD loaders, is that ID normalization is normalizing into an ID space. Then locate translates that ID a file path/URL, and that is where baseURL comes into play.

So for me, it does not make sense for normalized IDs to start with at './' since the IDs are already normalized to the "top" of the ID space. baseURL should not come into play at all for normalized IDs unless a path is needed to fetch the file that may contain a module with that ID.

jrburke avatar Apr 21 '14 17:04 jrburke

A significant difference in the es case: both import statements and import() functions. Former are compiler normalized.; latter are not. As you have noted previously, this is not a good API. If we are stuck with not-good can we make it not horrible? Many uses of import() will be at baseurl to start loading. For these cases arguments starting with './' will be used, because that is what we use for import statements. If we either use './' normal names or normalize wrt base URL, that argument will be correct. Not horrible for a common case. On Apr 21, 2014 10:30 AM, "James Burke" [email protected] wrote:

My understanding, and how it works in AMD loaders, is that ID normalization is normalizing into an ID space. Then locate translates that ID a file path/URL, and that is where baseURL comes into play.

So for me, it does not make sense for normalized IDs to start with at './' since the IDs are already normalized to the "top" of the ID space. baseURL should not come into play at all for normalized IDs unless a path is needed to fetch the file that may contain a module with that ID.

— Reply to this email directly or view it on GitHubhttps://github.com/jorendorff/js-loaders/issues/105#issuecomment-40955331 .

johnjbarton avatar Apr 21 '14 18:04 johnjbarton

@jrburke thanks - it seems we are in agreement here.

@johnjbarton from the discussions I've had with @wycats I believe the plan is to ensure System.import implements normalization equivalently to the import statement. This is certainly a necessity, and will be getting into the spec, even if I need to kick and scream at certain points.

guybedford avatar Apr 21 '14 19:04 guybedford

@johnjbarton my apologies perhaps not the best language. But even if import were not to implement normalization, we can still patch the behaviour.

guybedford avatar Apr 21 '14 19:04 guybedford

I like to refer to this as two separate concepts, in one hand we have names, and normalization of names, and in the other hand we have paths and the corresponding normalization and completion of those paths, where they just happen to have a similar structure, but that's just a coincidence. E.g.:

a) foo/bar/baz module that depends on ../xyz/abc, which are names to be normalized to foo/bar and foo/xyz/abc.

b) foo/bar/baz and foo/xyz/abc to be localized, based on baseURL or whatever loader is meant to do with those normalized names.

To be more specific, if we think about b) for a nodejs runtime on windows, it will normalize and complete to c://path//to//app//foo//bar//baz.js and so on, while a) will remain the same independent of the fact that in windows paths are normalized differently.

I think it is important to understand those distinctions. Ideally, System.import('./foo') will normalize to name foo, just like System.import('../foo') when executed at the top level, or maybe throwing an error if there is not parent module to normalize ../foo, but this should not leak to the normalization of the path during locate. IMO, that should be a complete separate process.

caridy avatar Apr 21 '14 19:04 caridy

Luckily the distance between normalized names and paths can be short: prefix with baseURL and postfix with '.js'. This allows developers to reason about the module names by comparing them to directory paths. Of course locate() can apply an arbitrary transformation if the developer feels insufficiently challenged during module development.

On Mon, Apr 21, 2014 at 12:54 PM, Caridy Patino [email protected]:

I like to refer to this as two separate concepts, in one hand we have names, and normalization of names, and in the other hand we have paths and the corresponding normalization and completion of those paths, where they just happen to have a similar structure, but that's just a coincidence. E.g.:

a) foo/bar/baz module that depends on ../xyz/abc, which are names to be normalized to foo/bar and foo/xyz/abc.

b) foo/bar/baz and foo/xyz/abc to be localized, based on baseURL or whatever loader is meant to do with those normalized names.

To be more specific, if we think about b) for a nodejs runtime on windows, it will normalize and complete to c://path//to//app//foo//bar//baz.js and so on, while a) will remain the same independent of the fact that in windows paths are normalized differently.

I think it is important to understand those distinctions. Ideally, System.import('./foo') will normalize to name foo, just like System.import('../foo') when executed at the top level, or maybe throwing an error if there is not parent module to normalize ../foo, but this should not leak to the normalization of the path during locate. IMO, that should be a complete separate process.

— Reply to this email directly or view it on GitHubhttps://github.com/jorendorff/js-loaders/issues/105#issuecomment-40970069 .

johnjbarton avatar Apr 21 '14 21:04 johnjbarton