jorendorff / js-loaders

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

Allow baseURL to be itself page-relative #105

Open guybedford opened 10 years ago

guybedford commented 10 years ago

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 commented 10 years ago

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

  toAbsoluteURL(toAbsoluteURL(document.baseURI, this.baseURL), address + '.js')
guybedford commented 10 years ago

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

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.

caridy commented 10 years ago

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

guybedford commented 10 years ago

@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.

caridy commented 10 years ago

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.

guybedford commented 10 years ago

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.

johnjbarton commented 10 years ago

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).

guybedford commented 10 years ago

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 notifications@github.com 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 .

johnjbarton commented 10 years ago

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 traceur@0.0.2/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?

guybedford commented 10 years ago

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

Names like traceur@0.0.2/src/options always resolve to the baseURL, becoming baseURL/traceur@0.0.2/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.

johnjbarton commented 10 years ago

@guybedford now I am really confused.

I though the idea of traceur@0.0.2/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.

guybedford commented 10 years ago

@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.

johnjbarton commented 10 years ago

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.

guybedford commented 10 years ago

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.

johnjbarton commented 10 years ago

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.

guybedford commented 10 years ago

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').

johnjbarton commented 10 years ago

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.

guybedford commented 10 years ago

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.

jrburke commented 10 years ago

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.

johnjbarton commented 10 years ago

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" notifications@github.com 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 .

guybedford commented 10 years ago

@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 commented 10 years ago

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

caridy commented 10 years ago

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.

johnjbarton commented 10 years ago

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 notifications@github.comwrote:

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 .