Skip to content

RFC to deprecate resetNamespace - #221

Closed
cibernox wants to merge 3 commits into
emberjs:masterfrom
cibernox:deprecate_reset_namespace
Closed

RFC to deprecate resetNamespace#221
cibernox wants to merge 3 commits into
emberjs:masterfrom
cibernox:deprecate_reset_namespace

Conversation

@cibernox

@cibernox cibernox commented Apr 18, 2017

Copy link
Copy Markdown
Contributor

@cibernox
cibernox force-pushed the deprecate_reset_namespace branch from 4ad8f97 to e062c21 Compare April 18, 2017 13:42
However, reseting the namespace of a route prevents developers to intuitively understand
the placement of a route in the nesting hierarchy of the app by simply reading its name,
making the mental model and the task of finding a a route in the file system more complex.
This issue is going to be amplificated by the deeply nested file structure of the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worse than that, because of local/global lookup.

@simonihmig

Copy link
Copy Markdown
Contributor

We have used resetNamespace more frequently. For most parts I would not have a problem with getting rid of it, but for one use case I find it actually very helpful.

That is when having a multilanguage site, we have a language route with a locale param that is used to set the locale of e.g. ember-i18n. All other routes are then children of this one. So you have URLs like "/en/foo" or "/de/foo". With resetNamespace you can then link to this route with {{#link-to "foo"}} instead of {{#link-to "language.foo"}}.

In this case the language. prefix of the route name is pretty much redundant, as it does not convey any information to the developer, i.e. not really helping with the mental model, as every route is a child of language. Also helps with making the change to support many languages later, as you can just nest the existing routes to a the new language route and reset their namespace, so all existing link-to/transitionToRoute continue to work.

@cibernox

Copy link
Copy Markdown
Contributor Author

@simonihmig Your use case is the only acceptable one, but still it just saves you from typing a few chars. In the case of my apps the omnipresent route is authenticated.
I still think that simplifying concepts is worth a few keystrokes. Perhaps a codemod could make the migration automatic?

@simonihmig

Copy link
Copy Markdown
Contributor

I am not too worried about keystrokes or migration (find and replace in your IDE should do most of it, and decent acceptance test coverage should catch the lost ones. Slightly off topic, but I am not aware of any codemod tools that can easily run on a Handlebars AST, you? As most occurrences of route names are probably with link-to in templates...)

I am more interested to retain the better readability, that you gain by hiding that omnipresent route IMHO. So I am still slightly in favor of having that escape hatch for those special cases that you and me mentioned. But in general I agree, this should not be abused for "normal" routes.

@mmun

mmun commented Apr 19, 2017

Copy link
Copy Markdown
Member

In principle I'm not opposed to this. However, the apps I work on use resetNamespace a lot. Otherwise we'd have routes link-to's like

{{#link-to 'headquarters.headquater.on-call-captains.captain' ...}}

There is cognitive overhead to thinking about the entire path of routes instead of just the terminal resource you're interested in. The RFC should describe why this is less important than what is in the "Motivation" section currently.

The "How We Teach This" section is not extensive enough. I'm not OK with link-to's that long. So you should explain how to avoid them.

EDIT: Another practical issue is that deep folder hierarchies take up horizontal space in your editor's file manager.

@cursedquail

Copy link
Copy Markdown

@mmun The only way out that I can see is engines, but for many apps the degree of encapsulation that engines require is infeasible

@workmanw

Copy link
Copy Markdown

I've found this feature to be quite useful for both of the reasons @mmun mentioned. Moreover, I'd like to hear more about why this feature is so dangerous that it should be removed. To me, this feature mostly amounts to scoping which concept present in many languages and frameworks. Module Unification even introduces the concept of "local lookup" which is another form of scoping.

@david-duncan

Copy link
Copy Markdown

We use this way too much in a way quite similar to @simonihmig

The grand majority of our routes are namespaced under an account (contextually like @cibernox mentioned of nesting under authenticated)

Used thousands of times across an app I think it saves us quite a bit....

I'm not seeing any real dangers listed other than it being a potential footgun / making apps hard to grok.

@courajs

courajs commented Apr 20, 2017

Copy link
Copy Markdown

In @ef4's EmberConf talk (around 36:00) he showed using resetNamespce in a 3rd-party route map.

@ef4

ef4 commented Apr 20, 2017

Copy link
Copy Markdown
Contributor

Yes. I think the path to not needing that is making engines configurable/composable enough to make third-party engines practical. Until then, resetNamespace is the only clean way to mount third-party routes in an arbitrary place.

"Third party routes" might sounds like an exotic and unusual thing to do, but I believe it's only unusual today because it's hard.

@kellyselden

Copy link
Copy Markdown
Member

I had a use case recently. resetNamespace makes it easy to go back and forth between using a rootURL and a top-level route that simulates the rootURL without moving all the files. This may be a temporary use case while you are testing out different server options, but it was legitimate for me.

@Gaurav0

Gaurav0 commented Apr 25, 2017

Copy link
Copy Markdown
Contributor

Actually, my most common use case is not mentioned above. I use it when I need the same subroute under more than one route. This is kind of similar to the third party route issue, just isn't third party.

all places where that route was referenced by its name (mostly `{{link-to}}`, but not exclusively).

However, reseting the namespace of a route prevents developers to intuitively understand
the placement of a route in the nesting hierarchy of the app by simply reading its name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be easier to read by having resetNamespace on by default?

What I mean is, this way this.route('deeply.nested.route') will always be an absolute path rather than a relative path to the parent route.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the router was first introduced, resetNamespace: true was the default behavior. People really disliked that you had to keep repeating yourself if you wanted to have nicely organized routes on disk.

@rwjblue

rwjblue commented Jan 22, 2018

Copy link
Copy Markdown
Member

I personally think we should eventually deprecate resetNamespace (as suggested here in this RFC) but the number of real world applications that still rely on it is quite large. Using resetNamespace is roughly the only way to leverage an addon to share routes / templates and have them be in a consistent location. That concern will hopefully be addressable once module unification lands, but in the meantime I don't think we can remove the feature...

Based on the conversation here, and the current status of route sharing in addons (including engines) I believe that we should close this for now, but we should absolutely still be working towards a future where it is possible to submit a new RFC along these same lines...

@rwjblue rwjblue closed this Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.