allow relative links

[akatov]

In ember-cli, when the rootURL is null it ends up being interpreted as an empty string.
This way we can achieve the same thing here.

See also #58

[san650]

@akatov Thanks for reporting the issue and push a fix!!

You need to update the tests to take care of this new behavior, can you do that? Please let me know if you need help!

[tschoartschi]

@akatov and @san650 I would need this functionality. What's the current status of this PR? I could help if someone gives me a status. Thanks a lot :)

[san650]

@akatov can you open an issue describing what's the exact problem? Maybe with an example of what configuration makes the addon to fail.

After that, we could re-do this PR and update/add the tests.

[tschoartschi]

@akatov I use ember-web-app with glimmer.js and my glimmer app is not in the root folder of my webserver, but with glimmer.js there is no need to define a rootURL. But now ember-web-app always resets the path to the icons etc to the root folder of the webserver. This happens with the following check:

var rootURL = config.rootURL || '/';

as @akatov mentioned in his PR the following would fix the problem:

var rootURL = config.rootURL || '';

This would also solve the problem for me. Did this make sense? Did I explained it well enough?

[san650]

@tschoartschi thanks for giving more details about the issue. I didn't know that about glimmer.js

I'll convert your comment into an issue and think on a solution. I want to check first if changing this

var rootURL = config.rootURL || '/';

to this

var rootURL = config.rootURL || '';

doesn't break existing applications or expected behavior for other people.

[san650]

I've converted this discussion into the issue #75. @tschoartschi do you want to work on a fix for this issue?

[tschoartschi]

I think the fix of @akatov is fine. Only the tests need to be adopted. But as you said, I'm not sure if this breaks any existing behaviour? I think every Ember-App has a rootURL defined, but if someone deleted the rootURL property in environment.js this change would lead to a different behaviour for this person. Another way would be to use some check like

var rootURL = (config.rootURL === undefined || config.rootURL === null) ? '/' : config.rootURL;

Then I could define the rootURL as empty string in my environment.js and everything would be fine. But I think this is kind of a hackish solution. But I think both have advantages and drawbacks. I would rather go with the solution @akatov provided, except there is a valid reason for Ember users to remove rootURL from their environment.js

Let me know what you think. Then I could create a fix 😃

[san650]

@tschoartschi let's go with @akatov solution. We can release this change as a major change so people upgrades taking this into account.

[tschoartschi]

@san650 cool 😃 do you have push rights to the PR of @akatov ? I think I don't have the rights to change this PR so I can't fix the tests. I don't want to create a new PR with the same changes only because I have no access rights.

[san650]

@tschoartschi I don't. Just start a fresh PR. We need to change rootUrl in more places now I think and update the tests.

I'll try to think on how to add a dummy glimmer app for the tests in the future, so we can validate that also. If you have any idea on how to achieve this or if there's any other addon that does this, that would be really useful.

[akatov]

@san650 @tschoartschi - I just gave you access to my fork of the repo, so you can update this PR directly

[akatov]

ah, sorry, just saw #76 :-)

[tschoartschi]

@akatov thanks a lot but it was quicker to just make a fresh PR :-) @san650 I created a PR #76, hope you like it ;-)

About the Glimmer.js dummy app: I'm not quite sure if it makes sense to already create a Glimmer.js test since I think there is still a lot in flux. Maybe you ask the guys in the slack channel #-glimmerjs first.