ember-cli / ember-cli/eslint-plugin-ember

encouraging snake_case dynamic segments results in routes erroring upon creation

Open
#410 0 comments 5 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
263
Forks
214
Avg merge
30m
Merged PRs (30d)
5

Description

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

3. 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...)

4. The `snake_case` default clashes with the enabled-by-default [`camelcase` eslint rule](https://eslint.org/docs/rules/camelcase) in places like this
```js
class Route {
model(params) {
const { book_id } = params;
}
}

```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.