fix:add starting slash to file-urls

[torbs]

Currently we will receive a url without a slash delimiter if we pass inn a relative url to the file method without a starting slash. With this fix the URL is correct even if the starting slash is missing from the argument. It does not break any of the current tests, but we should consider if this is a breaking change.

[digitalsadhu]

I agree that how it currently works is a bit of a footgun. Have run into it myself. Interested to hear @trygve-lie thoughts.

[digitalsadhu]

Any reason not to use path.isAbsolute here?

[torbs]

Just checked. isAbsolute is not doing the same thing as my test. my check is for an absolute URL, but path.isAbsolute checks for an absolute path. So isAbsolute('/test') is true but ABSOLUTE_URL_REGEX.test('/test') is false

[digitalsadhu]

Ah good point.

[torbs]

An alternative solution to this problem is:
value = `${base}${join('/', file)}`;
This is of course much simpler. But I believe using built-in methods to create the file path might help us avoid some edge cases

[trygve-lie]

I am fine with this. This did though get me to realize that we have more or less forgot to apply our common pattern for building paths and URL in this module. We try to strive for a pattern where each URL / path part always start with a / and never ends with one. We then wash argument on public methods for this and we then kinda assume we can just join them together without checking what they are everywhere.

We already have methods for this in the common package: https://github.com/eik-lib/common/blob/next/packages/utils/src/path-slashes.js

If we followed this pattern we would end up with something like so;

file(file = '') {
    const sanitizedFile = utils.addLeadingSlash(file);
    return new Asset({
        value: `${this.base()}${sanitizedFile}`,
    });
}

In the above return value from this.base() is safe because a removal of any end / is dealt with in that method.

Though; I think that is food for a second PR so I am fine with this.