Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaks locationType: "auto" #74

Closed
rlivsey opened this issue Sep 28, 2014 · 8 comments · Fixed by #75
Closed

breaks locationType: "auto" #74

rlivsey opened this issue Sep 28, 2014 · 8 comments · Fixed by #75

Comments

@rlivsey
Copy link
Member

rlivsey commented Sep 28, 2014

Just upgraded to master and locationType "auto" is ignored and gets set to "hash" for some reason.

The only change I made was upgrading, then downgrading back to 0.8.4 fixed it but @ef4 notes in #69 that it doesn't happen in a fresh app so I'll dig a little deeper.

@rlivsey
Copy link
Member Author

rlivsey commented Sep 28, 2014

I just replicated it in a new ember-cli app (0.0.46).

Here's the repo: https://github.com/rlivsey/liquid-fire-test

There's a console.log in router.js which just outputs the ember config. Note that it contains locationType: "hash".

If you remove liquid-fire (see/clone the without branch) then restart you'll see it is now locationType: "auto".

The only difference between these is the inclusion of liquid-fire.

I'll dig some more and see if I can figure out what commit causes it.

@ef4
Copy link
Collaborator

ef4 commented Sep 28, 2014

Thanks, with that repo I can reproduce.

@ef4
Copy link
Collaborator

ef4 commented Sep 28, 2014

The locationType value from liquid-fire's own config/environment.js is leaking through. I'm going to file an ember-cli bug and see what they say.

@ef4
Copy link
Collaborator

ef4 commented Sep 28, 2014

(Looks like it may be an intentional feature now that addons can export config, but it still seems wrong that the addon would be able to override the application.)

@rlivsey
Copy link
Member Author

rlivsey commented Sep 28, 2014

read my mind, was just in the process of testing that out!

@rlivsey
Copy link
Member Author

rlivsey commented Sep 28, 2014

Just confirmed that's definitely the issue.

See forked branch of the repo which uses a fork of liquid-fire with the config changed.

Cheers.

@rwjblue
Copy link
Contributor

rwjblue commented Sep 29, 2014

I submitted #75 to fix the immediate issue, but am also looking to figure out how/why the addon's config was overriding the applications config (app should ALWAYS win).

@ef4 ef4 closed this as completed in #75 Sep 29, 2014
@rwjblue
Copy link
Contributor

rwjblue commented Sep 29, 2014

Thanks for reporting this on ember-cli, there was definitely a bug that allowed the addon to override application config. ember-cli/ember-cli#2133 fixes the underlying problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants