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

New rule: `require-router-refresh-route-name`

Open
#1,758 0 comments 0 reactions 0 assignees View on GitHub
enhancement New Rule
Dominant language
JavaScript
Stars
263
Forks
214
Avg merge
30m
Merged PRs (30d)
5

Description

[RFC 631](https://rfcs.emberjs.com/id/0631-refresh-method-for-router-service) introduced the `refresh` to the RouterService class. With it, users can rerun the model hooks of (specific) active routes.

If the user doesn't pass in a route name the whole active route tree will be refreshed. This seemed fine at first, but what didn't occur to me immediately is that this will also rerun the model hooks of the _application_ route.

A lot of apps are using these hooks to run certain setup tasks (analytics, intl, authentication, ..) under the assumption that it will only ever be executed once (since you never leave the application route). If those apps run `this.router.refresh()` those setup tasks will also run again. While this might not always be a problem I still think that running refresh without an explicit route name is a bit of a footgun.

I think it would be better to require that the route name is always provided to the refresh method. That would prevent people from simply calling `this.router.refresh()` without thinking about the pivot route. People who do want to refresh the whole tree (including the application route) can still call `this.router.refresh('application')`. This has the advantage that the intention is more explicit.

A linting rule which would require the route name argument would be a nice addition. I'm not sure if this rule would be controversial but I'm fine with it not being part of the recommended ruleset if that's the case.

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.