Skip to content
This repository has been archived by the owner on Nov 23, 2024. It is now read-only.

New Linter Rule - set() must return a value #232

Closed
caseywatts opened this issue Mar 26, 2018 · 6 comments
Closed

New Linter Rule - set() must return a value #232

caseywatts opened this issue Mar 26, 2018 · 6 comments

Comments

@caseywatts
Copy link

having a linter rule for this would help prevent some bugs
(at least until the behavior changes to be what people expect)
emberjs/rfcs#79

@Turbo87
Copy link
Member

Turbo87 commented Mar 26, 2018

@caseywatts can you provide some examples of good and bad code?

@caseywatts
Copy link
Author

Sure! :)

Bad:

height: 100,
goldenRatioWidth: computed('height', {
  get() {
    return this.get('height') * 1.618;
  },
  set(_, value) {
    this.set('height', value / 1.618);
  }
})

Good

height: 100,
goldenRatioWidth: computed('height', {
  get() {
    return this.get('height') * 1.618;
  },
  set(_, value) {
    const newHeight = value / 1.618;
    this.set('height', newHeight);
    return newHeight;
  }
})

set() must have a return.

@Turbo87
Copy link
Member

Turbo87 commented Mar 26, 2018

@caseywatts seems like a good idea to me. do you want to work on it?

@Turbo87
Copy link
Member

Turbo87 commented Mar 26, 2018

oh, actually I just noticed what repository we're in. could you move the issue including the snippets to https://github.com/ember-cli/eslint-plugin-ember?

@caseywatts
Copy link
Author

ahaha you're right @Turbo87 thanks for catching this - I'll move this over to there 👍

@caseywatts
Copy link
Author

made this over at ember-cli/eslint-plugin-ember#247

Turbo87 pushed a commit that referenced this issue Oct 15, 2018
Bumps [ember-resolver](https://github.com/ember-cli/ember-resolver) from 4.1.0 to 5.0.1.
<details>
<summary>Changelog</summary>

*Sourced from [ember-resolver's changelog](https://github.com/ember-cli/ember-resolver/blob/master/CHANGELOG.md).*

> # Change Log
> 
> ## [v4.5.6](https://github.com/ember-cli/ember-resolver/tree/v4.5.6) (2018-06-13)
> [Full Changelog](ember-cli/ember-resolver@v4.5.5...v4.5.6)
> 
> **Closed issues:**
> 
> - An error occurred in the constructor for ember-resolver [\#234](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/234)
> - emberResolverFeatureFlags\(\) calls project.config\(\) without an environment name [\#233](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/233)
> - ember-cli-react/resolver not found [\#231](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/231)
> - Add ember-cli-eslint [\#200](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/200)
> 
> **Merged pull requests:**
> 
> - cleanup yo [\#240](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/240) ([stefanpenner](https://github.com/stefanpenner))
> - Added config type to module unification config [\#239](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/239) ([chrism](https://github.com/chrism))
> - use flag EMBER\_CLI\_MODULE\_UNIFICATION [\#238](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/238) ([givanse](https://github.com/givanse))
> - Fixup linting issues. [\#236](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/236) ([rwjblue](https://github.com/rwjblue))
> - Update to [email protected] blueprint. [\#235](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/235) ([rwjblue](https://github.com/rwjblue))
> 
> ## [v4.5.5](https://github.com/ember-cli/ember-resolver/tree/v4.5.5) (2018-03-23)
> [Full Changelog](ember-cli/ember-resolver@v4.5.4...v4.5.5)
> 
> **Closed issues:**
> 
> - \[4.5.3\] Build error [\#227](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/227)
> - Support for RFC 297 - Deprecation of Ember.Logger [\#223](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/223)
> 
> **Merged pull requests:**
> 
> - \[RFC 297\] Updated to conditionally use console rather than using Ember.Logger [\#232](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/232) ([lupestro](https://github.com/lupestro))
> - Update module unification spec link [\#230](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/230) ([josemarluedke](https://github.com/josemarluedke))
> - Use build-time `project.isModuleUnification\(\)` instead of feature flag. [\#228](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/228) ([cibernox](https://github.com/cibernox))
> 
> ## [v4.5.4](https://github.com/ember-cli/ember-resolver/tree/v4.5.4) (2018-03-09)
> [Full Changelog](ember-cli/ember-resolver@v4.5.3...v4.5.4)
> 
> ## [v4.5.3](https://github.com/ember-cli/ember-resolver/tree/v4.5.3) (2018-03-09)
> [Full Changelog](ember-cli/ember-resolver@v4.5.2...v4.5.3)
> 
> **Closed issues:**
> 
> - Namespaces [\#214](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/214)
> 
> **Merged pull requests:**
> 
> - Update MU trees: template-options is now template-compiler [\#226](https://github-redirect.dependabot.com/ember-cli/ember-resolver/pull/226) ([cibernox](https://github.com/cibernox))
> 
> ## [v4.5.2](https://github.com/ember-cli/ember-resolver/tree/v4.5.2) (2018-03-05)
> [Full Changelog](ember-cli/ember-resolver@v4.5.1...v4.5.2)
></table> ... (truncated)
</details>
<details>
<summary>Commits</summary>

- [`6303c48`](ember-cli/ember-resolver@6303c48) release v5.0.1 🎉
- [`0ffc530`](ember-cli/ember-resolver@0ffc530) Merge pull request [#243](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/243) from ember-cli/remove-default-resolver
- [`51f92e8`](ember-cli/ember-resolver@51f92e8) release v5.0.0 🎉
- [`df3ef4e`](ember-cli/ember-resolver@df3ef4e) Merge pull request [#244](https://github-redirect.dependabot.com/ember-cli/ember-resolver/issues/244) from ember-cli/chores
- [`1c40ceb`](ember-cli/ember-resolver@1c40ceb) Fix linting
- [`340f2ca`](ember-cli/ember-resolver@340f2ca) upgrade eslint + qunit-dom
- [`793ed30`](ember-cli/ember-resolver@793ed30) Disable not super important test for 3.3 test combat:
- [`777bdf1`](ember-cli/ember-resolver@777bdf1) upgrade CLI
- [`fa0302e`](ember-cli/ember-resolver@fa0302e) upgrades
- [`d63ac09`](ember-cli/ember-resolver@d63ac09) update lockfile (fix node 10 issue)
- Additional commits viewable in [compare view](ember-cli/ember-resolver@v4.1.0...v5.0.1)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=ember-resolver&package-manager=npm_and_yarn&previous-version=4.1.0&new-version=5.0.1)](https://dependabot.com/compatibility-score.html?dependency-name=ember-resolver&package-manager=npm_and_yarn&previous-version=4.1.0&new-version=5.0.1)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

Dependabot will **not** automatically merge this PR because it includes a major update to a development dependency.

---

**Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.

You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com).

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants