Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(eslint): disable function scoping #35

Merged
merged 5 commits into from
Jul 1, 2020
Merged

feat(eslint): disable function scoping #35

merged 5 commits into from
Jul 1, 2020

Conversation

stevensacks
Copy link
Contributor

This is a common pattern in React. The source of the measurements used as the basis for this rule were found to have been taken in the development version of React and were disproven. However, the myth persists. The React team has stated that this optimization is not necessary.

@stevensacks stevensacks requested a review from a team as a code owner May 11, 2020 20:20
JAdshead
JAdshead previously approved these changes May 11, 2020
@jsmartfo jsmartfo requested review from smackfu, drewcur and a team May 11, 2020 21:01
index.js Outdated
@@ -116,6 +116,10 @@ module.exports = {
// regular expressions.
'unicorn/no-unsafe-regex': 'error',

// This is a common pattern in React, and has been proven to be a premature optimization
Copy link
Contributor

Choose a reason for hiding this comment

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

can you provide a link for the "proven" portion? the rule itself has two links to support it's claims on perf, it'd be good to provide a counterargument if/when this rule comes into discussion in the future 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a warning would be sufficient? probably depends on the analysis?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't seem to find much supporting evidence either way

@github-actions
Copy link

This pull request is stale because it has been open 30 days with no activity.

@JAdshead JAdshead dismissed stale reviews from 10xLaCroixDrinker and themself via 7bc944e June 24, 2020 23:30
@JAdshead JAdshead changed the base branch from master to feature/v13 June 30, 2020 23:32
@JAdshead JAdshead dismissed PixnBits’s stale review July 1, 2020 15:36

updated to warn

@JAdshead JAdshead merged commit 344955e into americanexpress:feature/v13 Jul 1, 2020
JAdshead pushed a commit that referenced this pull request Jul 14, 2020
JAdshead pushed a commit that referenced this pull request Jul 16, 2020
oneamexbot added a commit that referenced this pull request Jul 16, 2020
# [13.0.0](v12.2.0...v13.0.0) (2020-07-16)

### Bug Fixes

* **eslint:** disable prevent abbreviations ([#33](#33)) ([6647bef](6647bef))

### Features

* **eslint:** configuration ([#30](#30)) ([6229d4a](6229d4a))
* **eslint:** disable function scoping ([#35](#35)) ([0a0c5bb](0a0c5bb))
* **eslint:** disable prefer default export ([#26](#26)) ([45b88d3](45b88d3))
* **eslint:** enable jest rules ([#29](#29)) ([846bbf7](846bbf7))
* **eslint:** react/jsx quality of life rules ([#24](#24)) ([facff92](facff92))
* **js:** additional js rules ([58df834](58df834))
* **prettier:** add ([#45](#45)) ([00cecaf](00cecaf))
* **unicorn:** update and seperate rules ([#43](#43)) ([940eb66](940eb66))
* **unicorn/consistent-function-scoping:** disable ([#42](#42)) ([54dd5a3](54dd5a3))

### BREAKING CHANGES

* **unicorn:** major update to unicorn
* **js:** no-lonely-if errors
* **js:** no-return-assign, errors
* **js:** prefer-object-spread, errors
* **js:** no-bitwise, errors
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants