[Bugfix] fixes bugs around baseURL and rootURL in HistoryLocation and introduces rootURL to NoneLocation #13520
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| rootURL = rootURL.replace(/\/$/, ''); | ||
|
|
||
| // remove rootURL from url | ||
| return path.replace(rootURL, ''); |
There was a problem hiding this comment.
Given a rootURL of /foo we want to strip it from /foo/bar/baz but we do not want /foo/baz/foo/bar to become /baz/bar.
There was a problem hiding this comment.
Yup I thought the same. This was taken from history location. Want me to fix both?
|
@rwjblue ready for another review -- thanks again |
jasonmit
changed the title
|
Thanks! |
|
@rwjblue we should likely force the rootURL to / when in testing. People likely have relied on this bug, I.e, a computed property as the rootURL or an initializer without checking the environment first. Thoughts? |
jasonmit
added a commit
to jasonmit/ember-cli
that referenced
this pull request
May 21, 2016
Now that this was merged emberjs/ember.js#13520 it's important people know that setting the `rootURL` on line 7 will likely cause some breakage in tests. Everyone likely relied on `rootURL` not implemented in `Ember.NoneLocation` as a feature instead of a bug.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently
rootURLisn't respected withinEmber.NoneLocationand the visit API which results in the route not to be found.The second bug,
formatURLalso does not return the url with therootURLprefixed. This breakslinktofrom building the proper string used in hrefs.This is blocking projects that rely on
rootURLto migrate to FastBoot.baseURLandrootURLfrom being string replaced at any point in the url forEmber.HistoryLocation