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.

Sign up for GitHub

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

Jump to bottom

ES6 (let&const): src/polyfills #19459

Merged
merged 2 commits into from
May 26, 2020
Merged

ES6 (let&const): src/polyfills #19459

merged 2 commits into from
May 26, 2020

Conversation

DefinitelyMaybe
Copy link
Contributor

related #19437

@DefinitelyMaybe DefinitelyMaybe changed the title replaced var with let src/polyfills: replaced var with let May 26, 2020
@gkjohnson
Copy link
Collaborator

Several of these can be set to const, right? source and output are never modified and I believe nextKey can be set to const in the for loop, as well. let is definitely the "safer" option here, though.

@DefinitelyMaybe
Copy link
Contributor Author

yup. gosh. I wasn't thinking on this one. one moment, I shall update the commit.

@DefinitelyMaybe DefinitelyMaybe changed the title src/polyfills: replaced var with let ES6 (let&const): src/polyfills May 26, 2020
@marcofugaro
Copy link
Contributor

marcofugaro commented May 26, 2020 •
edited
Loading

@DefinitelyMaybe setting the prefer-const and no-const-assign rules in the eslintConfig section in package.json might help you in the future.

They automatically detect if the variable is being mutated or not, assuming you're using VSCode 🙂

@mrdoob mrdoob added this to the r117 milestone May 26, 2020
@mrdoob mrdoob merged commit a84b7df into mrdoob:dev May 26, 2020
@mrdoob
Copy link
Owner

mrdoob commented May 26, 2020

Thanks!

@DefinitelyMaybe DefinitelyMaybe deleted the src/polyfills branch June 6, 2020 01:16
@DefinitelyMaybe DefinitelyMaybe mentioned this pull request Jul 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mugen87 Mugen87 Mugen87 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r117
Development

Successfully merging this pull request may close these issues.
5 participants
@DefinitelyMaybe @gkjohnson @marcofugaro @mrdoob @Mugen87