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

encouraging snake_case dynamic segments results in routes erroring upon creation #410

Open
mike-north opened this issue Apr 20, 2019 · 0 comments

Comments

@mike-north
Copy link
Member

mike-north commented Apr 20, 2019

Using the default lint configuration developers get from running ember new, after creating a route with the prescribed best practice of snake_case dynamic segment names (i.e., :foo_id) results in routes throwing errors immediately upon creation.

To Reproduce

ember new abc
cd abc
ember g route book --path="book/:book_id"
ember s
visit http://localhost:4200/book/42

You will get one of the following errors, depending on ember version

Error: No model was found for 'book'

or

Uncaught (in promise)
Error: Assertion Failed: You used the dynamic segment team_id in your route team,
but abc.Book did not exist and you did not override your route's `model` hook.

Suggested Solution

By encouraging developers to use snake_case dynamic segments in the lint rules, we're steering more developers into the path of the frequently-confusing default Route#model hook behavior (whereby, even in the absence of a custom model hook a snake_case dynamic segment name like :book_id, the store is queried for a record of type 'book' whose id is whatever value is found in the segment).

I think this should be an opt-in rule instead of being part of the default ruleset for the following reasons

  1. We're steering people toward a highly unintuitive rough spot in the framework (aforementioned model hook default behavior)

  2. The rationalization for this is rule is based on performance concerns around a router implementation detail (tbh I don't even know if this is true anymore?)

Dynamic segments in routes should use snake case, so Ember doesn't have to do extra serialization in order to resolve promises.

I haven't measured this, but I have a suspicion that even if there is a performance concern, it won't even be observable except in the very largest apps

  1. Once we put the alleged performance penalty aside, this is just a stylistic choice, and we should leave this kind of thing in the hands of developers (same with tabs vs. spaces, single vs. double quotes, etc...)

  2. The snake_case default clashes with the enabled-by-default camelcase eslint rule in places like this

class Route {
    model(params) {
        const { book_id } = params;
    }
}
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

No branches or pull requests

1 participant