Add eslint as linter tool by JuanMaRuiz · Pull Request #442 · jquery/sizzle

JuanMaRuiz · 2019-03-07T19:46:02Z

As discussed in the opened issue jscs is deprecated and jQuery is using eslint as linter tool. I have configured eslint as is in the jQuery project. But I have had to configure something in a special way: For example: I have marked as to skip the rule "no-nested-ternary"

  "rules": {
	"no-nested-ternary": "warn"
  }

because the style of coding of sizzle is not the same as the jQuery.

I have tried to extract to functions but I cannot name the functions with a clear name or because I think it could increase the complexity of the code. I have also tried to replace ternaries with ifs but it increase the weight of the build.

Please, let me know I this PR is useful and/or needs to be updated.

Best regards.

fix: #438

- Removed jscs and jshint: Config files and grunt tasks - Added eslint: Added dependencies and grunt task

- Configuring eslint: Using jquery configuration

- Configuring eslint: Using jquery configuration - Fix lint errors: Fixed errors using eslint configuraion

Fixing line length marked as error by eslint jquery configuration

Line 1676 -> error Do not nest ternary expressions no-nested-ternary

- Configured eslint with jQuery configuration for main project and speed folder - Fixed all lint errors - Marked as warning the rule not-nested-ternary fix: #438

mgol

Thanks for the PR! I left some comments to address.

- Reverted indentation of src/sizzle.js file. - Applied reviewer proposal except the use of strict mode fix: #438

JuanMaRuiz · 2019-03-07T23:08:29Z

Thanks for the PR! I left some comments to address.

Many thanks to you for the quick review!! I have applied almost all the request. If you have any time, could to please review it again?

Best regards.

- Added 'strict mode' (pending of review from reviewers) to fix ci fix: #438

…zzle into fix-438-use-eslint

- Reverting dot notation due Google compiler concern

- Fixed eslint error when reverting dot notation due Google compiler concern. - Disable line to avoid error for not using dot notation according eslint config

- Added eslint configuration files for test folder. - Added changes requested in PR #442

- Fixed all style issues detected by eslint using jquery configuration

- Added package-lock.json

JuanMaRuiz · 2019-05-07T18:54:09Z

Thanks for the PR! I left some comments to address.

Hi @mgol I have noted that I missed the configuration of some test and tasks files. I have tried to mimic the jquery configuration. I hope the configuration is the correct ones.

Please, if you have any time let me know if everything is ok now.

Best regards.

JuanMa.

mgol

See my newest comments.

- Reverted indentation of src/sizzle.js file. - Remove "use strict" form task files. - Added new eslint configuration files. - Configured lint and default grunt tasks as jquery project. fix: #438

mgol

Thanks for the update! I added more comments.

@mgol

- Added all [PR](#442) requested by @mgol fix: #438

JuanMaRuiz · 2019-05-26T15:37:40Z

Thanks for the update! I added more comments.

Many thanks for your quick review @mgol ! I hope is everything is fits the requirements now.

Best regards!

- Removed browser support in main .eslintrc.json fix: #438

- .eslintignore file configured properly to check sizzle file and added comment for clarification fix: #438

- Better comments indentation - Removed .jshintrc file from test/karma folder fix: #438

mgol

Thanks for all the work! It's getting close. See my newest comments (all the old threads are resolved now).

- Fixed some indentation in comments. - Changed some dot notations to bracket notation.

- Re-indented some comments and code after added eslint - Changed location and setTimeout for window.location and window.setTimeout. fix: #438

…-use-eslint

JuanMaRuiz · 2019-05-28T16:08:35Z

Thanks for all the work! It's getting close. See my newest comments (all the old threads are resolved now).

Great!! I have added all the changes you requested.

Best regards.

- Reverted dot notation to bracket notation in src/sizzle file fix: #438

mgol · 2019-05-28T21:41:02Z

@timmywil @gibson042 Do you want to review this as well? The PR LGTM so - if not - I'd land it soon.

mgol · 2019-05-29T15:05:07Z

I compared the minified file on master & on this branch & I found two bugs so it's good that I checked! I fixed them, see the screenshot for changes:

The first issue was forgetting a | when breaking a string into two lines. The second one was that the commit 7d92424 got reverted. After my fixes the only difference in the minified file is the version in the header that wasn't updated before so we should have no more issues. Phew!

mgol · 2019-05-29T15:05:40Z

BTW, especially the first issue means we're lacking test coverage in that area.

Newest eslint-config-jquery requires prefixing unused function parameters with and underscore; that has been done. The build Grunt task was also fixed to lint the built Sizzle version only after it's built. Ref gh-442

mgol · 2019-05-29T15:15:23Z

Landed, thanks @JuanMaRuiz for the whole effort!

JuanMaRuiz · 2019-05-29T15:37:21Z

Landed, thanks @JuanMaRuiz for the whole effort!

Thanks to you @mgol for your comments and review.

This is a resubmit of jquerygh-439 that was lost during the switch from JSHint + JSCS to ESLint Ref jquerygh-442 Ref jquerygh-439 (cherry picked from commit 7d92424)

With new selector code doing less convoluted support tests, it was possible to extract a lot of logic out of setDocument & also reduce size. This commit also backports jquery/sizzle#439 that was reverted by mistake during a switch from JSHint + JSCS to ESLint. Ref jquery/sizzle#442 Ref jquery/sizzle#439

This is a resubmit of jquerygh-439 that was lost during the switch from JSHint + JSCS to ESLint Ref jquerygh-442 Ref jquerygh-439 (cherry picked from commit 7d92424)

With new selector code doing less convoluted support tests, it was possible to extract a lot of logic out of setDocument & also reduce size. This commit also backports jquery/sizzle#439 that was reverted by mistake during a switch from JSHint + JSCS to ESLint. Ref jquery/sizzle#442 Ref jquery/sizzle#439

With new selector code doing less convoluted support tests, it was possible to extract a lot of logic out of setDocument & also reduce size. This commit also backports jquery/sizzle#439 that was reverted by mistake during a switch from JSHint + JSCS to ESLint. Closes gh-4462 Ref jquery/sizzle#442 Ref jquery/sizzle#439

JuanMaRuiz added 8 commits January 20, 2019 17:22

Build: Configure eslint

6315322

- Removed jscs and jshint: Config files and grunt tasks - Added eslint: Added dependencies and grunt task

Build: Configure eslint

54be4f2

- Configuring eslint: Using jquery configuration

Build: Configure eslint

ed18cae

- Configuring eslint: Using jquery configuration - Fix lint errors: Fixed errors using eslint configuraion

Build: Fixing eslint errors

4efc809

Fixing line length marked as error by eslint jquery configuration

Build: Fixing eslint line length errors

97f2bfe

Fixing line length marked as error by eslint jquery configuration

Build: Fixing eslint non nesting ternary

49d0a94

Line 1676 -> error Do not nest ternary expressions no-nested-ternary

Build: Removed old jshint grunt task

624e7ce

Build: Configure eslint

12ee0c5

- Configured eslint with jQuery configuration for main project and speed folder - Fixed all lint errors - Marked as warning the rule not-nested-ternary fix: #438

mgol requested changes Mar 7, 2019

View reviewed changes

Build: Configure eslint

13a8efa

- Reverted indentation of src/sizzle.js file. - Applied reviewer proposal except the use of strict mode fix: #438

JuanMaRuiz added 9 commits March 8, 2019 00:10

Build: Configure eslint

57c01b7

- Added 'strict mode' (pending of review from reviewers) to fix ci fix: #438

Build: Configure eslint

a80d39b

- Added 'strict mode' (pending of review from reviewers) to fix ci fix: #438

Merge branch 'fix-438-use-eslint' of https://github.com/JuanMaRuiz/si…

c888650

…zzle into fix-438-use-eslint

Build: Configure eslint - PR requested changes

5c763ff

- Reverting dot notation due Google compiler concern

Build: Configure eslint

4cc4ae1

- Fixed eslint error when reverting dot notation due Google compiler concern. - Disable line to avoid error for not using dot notation according eslint config

Build: Configure eslint for test folder

7b96d79

- Added eslint configuration files for test folder. - Added changes requested in PR #442

Merge branch 'master' into fix-438-use-eslint

e1e793e

Build: Configure eslint for test folder

57efcda

- Fixed all style issues detected by eslint using jquery configuration

Build: Configure eslint for test folder

03f498c

- Added package-lock.json

mgol requested changes May 13, 2019

View reviewed changes

Comment thread tasks/version.js Outdated

Comment thread src/sizzle.js Outdated

mgol reviewed May 13, 2019

View reviewed changes

Comment thread src/sizzle.js Outdated

Build: Configure eslint - PR request

8fae5dd

- Reverted indentation of src/sizzle.js file. - Remove "use strict" form task files. - Added new eslint configuration files. - Configured lint and default grunt tasks as jquery project. fix: #438

mgol reviewed May 25, 2019

View reviewed changes

mgol requested changes May 25, 2019

View reviewed changes

Build: Configure eslint - PR improvements

24d0a94

- Added all [PR](#442) requested by @mgol fix: #438

JuanMaRuiz added 2 commits May 26, 2019 17:39

Build: Configure eslint - PR improvements

684520f

- Removed browser support in main .eslintrc.json fix: #438

Build: Configure eslint - PR improvements

f258209

- .eslintignore file configured properly to check sizzle file and added comment for clarification fix: #438

Build: Improve eslint configuration, fix configuration errors

9f12b3b

- Better comments indentation - Removed .jshintrc file from test/karma folder fix: #438

mgol requested changes May 27, 2019

View reviewed changes

JuanMaRuiz added 3 commits May 27, 2019 23:42

Build: Some PR improvements and re-indents

09d6a26

- Fixed some indentation in comments. - Changed some dot notations to bracket notation.

Build: Improve eslint configuration, changed location and setTimeout

7c3f27e

- Re-indented some comments and code after added eslint - Changed location and setTimeout for window.location and window.setTimeout. fix: #438

Merge remote-tracking branch 'origin/fix-438-use-eslint' into fix-438…

bfe54cd

…-use-eslint

Build: Reverted dot notation to bracket notation in sizzle

e8a2553

- Reverted dot notation to bracket notation in src/sizzle file fix: #438

mgol approved these changes May 29, 2019

View reviewed changes

timmywil approved these changes May 29, 2019

View reviewed changes

mgol closed this in 62061d0 May 29, 2019

JuanMaRuiz deleted the fix-438-use-eslint branch May 29, 2019 15:37

mgol mentioned this pull request Aug 19, 2019

Core: Throw a better error when documentElement is missing #452

Closed

mgol added this to the 2.3.5 milestone Feb 28, 2020

mgol mentioned this pull request Feb 28, 2020

Core: Throws an error if the document element is null #439

Merged

Conversation

JuanMaRuiz commented Mar 7, 2019

Uh oh!

mgol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JuanMaRuiz commented Mar 7, 2019

Uh oh!

JuanMaRuiz commented May 7, 2019

Uh oh!

mgol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mgol left a comment

Choose a reason for hiding this comment

Uh oh!

JuanMaRuiz commented May 26, 2019

Uh oh!

mgol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JuanMaRuiz commented May 28, 2019

Uh oh!

mgol commented May 28, 2019

Uh oh!

mgol commented May 29, 2019

Uh oh!

mgol commented May 29, 2019

Uh oh!

mgol commented May 29, 2019

Uh oh!

JuanMaRuiz commented May 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants