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

Breaking test: resetNamespace _within_ an engine's routes is broken #803

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

machty
Copy link

@machty machty commented Mar 22, 2022

Using resetNamespace:true when mounting an engine works, but if the engine itself has routes with resetNamespace:true, those routes get entirely stripped of their engine mount prefix, causing errors within link-to and any other place that needs to resolve the route name.

In this PR, post-reset-namespace should have a route name of blog.post-reset-namespace but it gets registered (potentially multiple times if there are multiple mounts) as just post-reset-namespace, causing errors like:

Error: Assertion Failed: You attempted to generate a link for the "post-reset-namespace" route, but did not pass the models required for generating its dynamic segments. There is no route named blog.post-reset-namespace

I suspect the fix might need to be in Ember proper, but this seemed like the right place to share.

@machty machty changed the title Breaking test: resetNamespace _within_ an engines routes is broken Breaking test: resetNamespace _within_ an engine's routes is broken Mar 22, 2022
@runspired
Copy link
Contributor

runspired commented Apr 19, 2022

I think I agree here, resetNamespace should consider the current engine (the root app in the case of an app, which is in fact also implemented as an engine!) as the root, not the app root. I believe this would be a fix in either router.js or ember-source.

Not fixing this leaks the isolation primitive and results in a high potential for conflict, honestly just not great all around. So hopefully we can do that.

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 this pull request may close these issues.

None yet

2 participants