search

jquery / sizzle

A sizzlin' hot selector engine.
https://sizzlejs.com
Other
6.29k stars 951 forks source link

jscs has been deprecated #438

Closed JuanMaRuiz closed 5 years ago

JuanMaRuiz commented 5 years ago

jscss has been deprecated. Maybe it's time to start using a new linter tool. What do you think about it? What is your preferred linter tool?

Best regards.

mgol commented 5 years ago

Thanks for the report. We're aware of JSCS deprecation and would ideally switch to ESLint with eslint-config-jquery as jQuery itself has done a while ago.

That said, we're not sure how long Sizzle is going to live as we'd like to dump it for jQuery 4.0 if possible, that's one of the reasons why no one put effort in trying to migrate to ESLint.

I think we'd welcome a move to ESLint if you're interested to try one. Correct me, @gibson042, if that's not the case.

JuanMaRuiz commented 5 years ago

Many thanks for your answer @mgol I'll wait @gibson042 answer's. Let me know if I can help with that if sizzle is going to keep alive.

Best regards.

gibson042 commented 5 years ago

Seems good to me. I haven't yet submitted a patch to jQuery for abandoning Sizzle, so it lives by default for now.

JuanMaRuiz commented 5 years ago

Ok. Then I'll try to do it.

Best regards.

May thanks @gibson042 @mgol

JuanMaRuiz commented 5 years ago

Hi @mgol and @gibson042 I have created a fork from sizzle and I have started to work in the implementation of eslint in the project. I have some doubts and I don't know how to proceed with them.

First of all I have configured the project to use the official eslint configuration of jquery (as @mgol said in a previous comment).

I have fixed some of the style errors (most of them related with the max line length. But there are some errors that I don't know how do you think I should resolve. That errors are about no-nested-ternary property.

Two approaches here:

As you can see there are a lot of them in the code: no-nested-ternary-errors-sizzle

What do you think about it? What is your preferred solution?

Best regards.

mgol commented 5 years ago

I think it would work better as a series of ifs, provided that it can be achieved without negative effect on gzipped minified file size (the build reports that in the end).

JuanMaRuiz commented 5 years ago

Many thanks @mgol I'll try in that way.

Best regards.

JuanMaRuiz commented 5 years ago

Hi @mgol I have tried to change all the ternaries with ifs but I have note that the build size is increased. I have create a PR. I have configured eslint to mark as warning the rule of nesting-ternary.

If you have any time, could you please check the PR and tell me if I have to do more changes and/or you are agree with the proposal??

Many thanks.

Best regards.

Drenmi commented 5 years ago

That said, we're not sure how long Sizzle is going to live as we'd like to dump it for jQuery 4.0 if possible, that's one of the reasons why no one put effort in trying to migrate to ESLint.

Hi, @mgol!

Could you elaborate a bit more on what the strategy is for 4.0? Are you putting the selector code in core, or creating a new library? Changing the support matrix, or just getting rid of old legacy? 🙂

mgol commented 5 years ago

@Drenmi We'll put the new selector code directly in jQuery. The manual DOM traversal will be removed in favor of using querySelectorAll directly. For cases where we detect browser bugs and skip qSA currently, we'll instead rewrite the selector to an equivalent one that's not buggy. For example, if a browser had a bug in document.querySelectorAll( "#my-id" ) we can rewrite it to document.querySelectorAll( "[id='my-id']" ). This will still require us to parse & tokenize the selector but we won't have to implement traversal so it should be much simpler. Also, we won't have to workaround issues in old browsers but only in ones supported by jQuery Core.