Permalink
...
timmywil
Timmy Willison
christianhaller
commented on
LaurentBarbareau
commented on
LaurentBarbareau
commented on
LaurentBarbareau
commented on
leinue
commented on
Comparing changes
Open a pull request
9
contributors
Commits on Jul 07, 2016
Commits on Jul 08, 2016
Commits on Jul 13, 2016
Commits on Jul 15, 2016
Commits on Jul 25, 2016
Commits on Aug 02, 2016
Commits on Aug 05, 2016
Commits on Aug 08, 2016
Commits on Aug 10, 2016
Commits on Aug 15, 2016
This commit was signed with a verified signature.
timmywil
Timmy Willison
Commits on Sep 12, 2016
Commits on Sep 15, 2016
Commits on Sep 19, 2016
Commits on Sep 22, 2016
Unified
Split
Showing
with
18,909 additions
and 316 deletions.
- 0 .eslintrc → .eslintrc.json
- +2 −2 .gitignore
- +4 −2 .mailmap
- +1 −1 .npmignore
- +1 −1 AUTHORS.txt
- +59 −26 Gruntfile.js
- +5 −4 build/release/cdn.js
- +4 −3 build/release/dist.js
- +2 −6 build/tasks/build.js
- +4 −2 dist/{.eslintrc → .eslintrc.json}
- +10,220 −0 dist/jquery.js
- +4 −0 dist/jquery.min.js
- +1 −0 dist/jquery.min.map
- +8,107 −0 dist/jquery.slim.js
- +4 −0 dist/jquery.slim.min.js
- +1 −0 dist/jquery.slim.min.map
- +88 −33 external/sizzle/dist/sizzle.js
- +2 −2 external/sizzle/dist/sizzle.min.js
- +1 −1 external/sizzle/dist/sizzle.min.map
- +14 −10 package.json
- 0 src/{.eslintrc → .eslintrc.json}
- +7 −7 src/ajax.js
- +3 −2 src/ajax/load.js
- +6 −3 src/attributes/attr.js
- +12 −17 src/attributes/classes.js
- +20 −5 src/attributes/prop.js
- +19 −15 src/attributes/val.js
- +3 −3 src/callbacks.js
- +8 −7 src/core.js
- +9 −6 src/core/access.js
- +4 −10 src/core/ready-no-deferred.js
- +14 −0 src/core/stripAndCollapse.js
- +9 −7 src/css.js
- +1 −1 src/css/showHide.js
- +26 −8 src/data.js
- +3 −3 src/data/Data.js
- +11 −6 src/effects.js
- +41 −22 src/event.js
- +6 −6 src/exports/global.js
- +3 −2 src/jquery.js
- +17 −9 src/manipulation/getAll.js
- +11 −7 src/serialize.js
- +19 −12 src/traversing/findFilter.js
- +8 −0 src/var/rnothtmlwhite.js
- +0 −5 src/var/rnotwhite.js
- +1 −1 src/wrapper.js
- +0 −1 test/{.eslintrc → .eslintrc.json}
- +1 −0 test/data/test3.html
- +1 −3 test/data/testinit.js
- +1 −1 test/node_smoke_tests/{.eslintrc → .eslintrc.json}
- +0 −4 test/promises_aplus_adapters/.eslintrc
- +4 −0 test/promises_aplus_adapters/.eslintrc.json
- +30 −9 test/unit/ajax.js
- +47 −3 test/unit/attributes.js
- +13 −17 test/unit/core.js
- +10 −1 test/unit/css.js
- +1 −1 test/unit/manipulation.js
- +2 −19 test/unit/support.js
- +14 −0 test/unit/traversing.js
View
0
.eslintrc → .eslintrc.json
File renamed without changes.
View
4
.gitignore
| @@ -8,11 +8,11 @@ | ||
| .bower.json | ||
| .sizecache.json | ||
| -npm-debug.log | ||
| +npm-debug.log* | ||
| # Ignore everything in dist folder except for eslint config | ||
| /dist/* | ||
| -!/dist/.eslintrc | ||
| +!/dist/.eslintrc.json | ||
| /node_modules | ||
View
6
.mailmap
| @@ -86,8 +86,10 @@ Scott Jehl <[email protected]> <[email protected]> | ||
| Sebastian Burkhard <[email protected]> | ||
| Senya Pugach <[email protected]> | ||
| Thomas Tortorini <[email protected]> Mr21 | ||
| -Timmy Willison <[email protected]> | ||
| -Timmy Willison <[email protected]> <[email protected]> | ||
| +Timmy Willison <[email protected]> | ||
| +Timmy Willison <[email protected]> <[email protected]> | ||
| +Timmy Willison <[email protected]> <[email protected]> | ||
| +Timmy Willison <[email protected]> <[email protected]> | ||
| Timo Tijhof <[email protected]> | ||
| TJ Holowaychuk <[email protected]> | ||
| Tom H Fuertes <[email protected]> | ||
View
2
.npmignore
| @@ -1,5 +1,5 @@ | ||
| .eslintignore | ||
| -.eslintrc | ||
| +.eslintrc.json | ||
| /.editorconfig | ||
| /.gitattributes | ||
View
2
AUTHORS.txt
| @@ -77,7 +77,7 @@ Jared Grippe <[email protected]> | ||
| Sylvester Keil <[email protected]> | ||
| Brandon Sterne <[email protected]> | ||
| Mathias Bynens <[email protected]> | ||
| -Timmy Willison <timmywillisn@gmail.com> | ||
| +Timmy Willison <4timmywil@gmail.com> | ||
| Corey Frang <[email protected]> | ||
| Digitalxero <digitalxero> | ||
| Anton Kovalyov <[email protected]> | ||
View
85
Gruntfile.js
| @@ -14,9 +14,14 @@ module.exports = function( grunt ) { | ||
| var fs = require( "fs" ), | ||
| gzip = require( "gzip-js" ), | ||
| + oldNode = /^v0\./.test( process.version ); | ||
| - // Skip jsdom-related tests in Node.js 0.10 & 0.12 | ||
| - runJsdomTests = !/^v0/.test( process.version ); | ||
| + // Support: Node.js <4 | ||
| + // Skip running tasks that dropped support for Node.js 0.10 & 0.12 | ||
| + // in those Node versions. | ||
| + function runIfNewNode( task ) { | ||
| + return oldNode ? "print_old_node_message:" + task : task; | ||
| + } | ||
| if ( !grunt.option( "filename" ) ) { | ||
| grunt.option( "filename", "jquery.js" ); | ||
| @@ -108,9 +113,15 @@ module.exports = function( grunt ) { | ||
| // See https://github.com/sindresorhus/grunt-eslint/issues/119 | ||
| quiet: true | ||
| }, | ||
| - all: ".", | ||
| - dist: "dist/jquery.js", | ||
| - dev: [ "src/**/*.js", "Gruntfile.js", "test/**/*.js", "build/**/*.js" ] | ||
| + | ||
| + // We have to explicitly declare "src" property otherwise "newer" | ||
| + // task wouldn't work properly :/ | ||
| + dist: { | ||
| + src: "dist/jquery.js" | ||
| + }, | ||
| + dev: { | ||
| + src: [ "src/**/*.js", "Gruntfile.js", "test/**/*.js", "build/**/*.js" ] | ||
| + } | ||
| }, | ||
| testswarm: { | ||
| tests: [ | ||
| @@ -144,7 +155,7 @@ module.exports = function( grunt ) { | ||
| ] | ||
| }, | ||
| watch: { | ||
| - files: [ "<%= eslint.dev %>" ], | ||
| + files: [ "<%= eslint.dev.src %>" ], | ||
| tasks: [ "dev" ] | ||
| }, | ||
| uglify: { | ||
| @@ -176,33 +187,55 @@ module.exports = function( grunt ) { | ||
| } ); | ||
| // Load grunt tasks from NPM packages | ||
| - require( "load-grunt-tasks" )( grunt ); | ||
| + // Support: Node.js <4 | ||
| + // Don't load the eslint task in old Node.js, it won't parse. | ||
| + require( "load-grunt-tasks" )( grunt, { | ||
| + pattern: oldNode ? [ "grunt-*", "!grunt-eslint" ] : [ "grunt-*" ] | ||
| + } ); | ||
| // Integrate jQuery specific tasks | ||
| grunt.loadTasks( "build/tasks" ); | ||
| - grunt.registerTask( "lint", [ "jsonlint", "eslint:all" ] ); | ||
| + grunt.registerTask( "print_old_node_message", function() { | ||
| + var task = [].slice.call( arguments ).join( ":" ); | ||
| + grunt.log.writeln( "Old Node.js detected, running the task \"" + task + "\" skipped..." ); | ||
| + } ); | ||
| - // Don't run Node-related tests in Node.js < 1.0.0 as they require an old | ||
| - // jsdom version that needs compiling, making it harder for people to compile | ||
| - // jQuery on Windows. (see gh-2519) | ||
| - grunt.registerTask( "test_fast", runJsdomTests ? [ "node_smoke_tests" ] : [] ); | ||
| + grunt.registerTask( "lint", [ | ||
| + "jsonlint", | ||
| + runIfNewNode( "eslint" ) | ||
| + ] ); | ||
| - grunt.registerTask( "test", [ "test_fast" ].concat( | ||
| - runJsdomTests ? [ "promises_aplus_tests" ] : [] | ||
| - ) ); | ||
| + grunt.registerTask( "lint:newer", [ | ||
| + "newer:jsonlint", | ||
| + runIfNewNode( "newer:eslint" ) | ||
| + ] ); | ||
| - // Short list as a high frequency watch task | ||
| - grunt.registerTask( "dev", [ | ||
| - "build:*:*", | ||
| - "newer:eslint:dev", | ||
| - "uglify", | ||
| - "remove_map_comment", | ||
| - "dist:*" | ||
| - ] | ||
| - ); | ||
| + grunt.registerTask( "test:fast", runIfNewNode( "node_smoke_tests" ) ); | ||
| + grunt.registerTask( "test:slow", runIfNewNode( "promises_aplus_tests" ) ); | ||
| - grunt.registerTask( "default", [ "dev", "eslint:dist", "test_fast", "compare_size" ] ); | ||
| + grunt.registerTask( "test", [ | ||
| + "test:fast", | ||
| + "test:slow" | ||
| + ] ); | ||
| - grunt.registerTask( "precommit_lint", [ "newer:jsonlint", "newer:eslint:all" ] ); | ||
| + grunt.registerTask( "dev", [ | ||
| + "build:*:*", | ||
| + runIfNewNode( "newer:eslint:dev" ), | ||
| + "newer:uglify", | ||
| + "remove_map_comment", | ||
| + "dist:*", | ||
| + "compare_size" | ||
| + ] ); | ||
| + | ||
| + grunt.registerTask( "default", [ | ||
| + runIfNewNode( "eslint:dev" ), | ||
| + "build:*:*", | ||
| + "uglify", | ||
| + "remove_map_comment", | ||
| + "dist:*", | ||
| + runIfNewNode( "eslint:dist" ), | ||
| + "test:fast", | ||
| + "compare_size" | ||
| + ] ); | ||
| }; | ||
View
9
build/release/cdn.js
| @@ -40,11 +40,12 @@ function makeReleaseCopies( Release ) { | ||
| // Map files need to reference the new uncompressed name; | ||
| // assume that all files reside in the same directory. | ||
| - // "file":"jquery.min.js","sources":["jquery.js"] | ||
| + // "file":"jquery.min.js" ... "sources":["jquery.js"] | ||
| text = fs.readFileSync( builtFile, "utf8" ) | ||
| - .replace( /"file":"([^"]+)","sources":\["([^"]+)"\]/, | ||
| - "\"file\":\"" + unpathedFile.replace( /\.min\.map/, ".min.js" ) + | ||
| - "\",\"sources\":[\"" + unpathedFile.replace( /\.min\.map/, ".js" ) + "\"]" ); | ||
| + .replace( /"file":"([^"]+)"/, | ||
| + "\"file\":\"" + unpathedFile.replace( /\.min\.map/, ".min.js\"" ) ) | ||
| + .replace( /"sources":\["([^"]+)"\]/, | ||
| + "\"sources\":[\"" + unpathedFile.replace( /\.min\.map/, ".js" ) + "\"]" ); | ||
| fs.writeFileSync( releaseFile, text ); | ||
| } else if ( builtFile !== releaseFile ) { | ||
| shell.cp( "-f", builtFile, releaseFile ); | ||
View
7
build/release/dist.js
| @@ -94,17 +94,18 @@ module.exports = function( Release, files, complete ) { | ||
| fs.writeFileSync( Release.dir.dist + "/bower.json", generateBower() ); | ||
| console.log( "Adding files to dist..." ); | ||
| - Release.exec( "git add .", "Error adding files." ); | ||
| + | ||
| + Release.exec( "git add -A", "Error adding files." ); | ||
| Release.exec( | ||
| - "git commit -m 'Release " + Release.newVersion + "'", | ||
| + "git commit -m \"Release " + Release.newVersion + "\"", | ||
| "Error committing files." | ||
| ); | ||
| console.log(); | ||
| console.log( "Tagging release on dist..." ); | ||
| Release.exec( "git tag " + Release.newVersion, | ||
| "Error tagging " + Release.newVersion + " on dist repo." ); | ||
| - Release.tagTime = Release.exec( "git log -1 --format='%ad'", | ||
| + Release.tagTime = Release.exec( "git log -1 --format=\"%ad\"", | ||
| "Error getting tag timestamp." ).trim(); | ||
| } | ||
View
8
build/tasks/build.js
| @@ -17,7 +17,6 @@ module.exports = function( grunt ) { | ||
| read = function( fileName ) { | ||
| return grunt.file.read( srcFolder + fileName ); | ||
| }, | ||
| - globals = read( "exports/global.js" ), | ||
| wrapper = read( "wrapper.js" ).split( /\/\/ \@CODE\n\/\/[^\n]+/ ), | ||
| config = { | ||
| baseUrl: "src", | ||
| @@ -38,11 +37,8 @@ module.exports = function( grunt ) { | ||
| // Avoid breaking semicolons inserted by r.js | ||
| skipSemiColonInsertion: true, | ||
| wrap: { | ||
| - start: wrapper[ 0 ].replace( /\/\*eslint .* \*\/\n/, "" ), | ||
| - end: globals.replace( | ||
| - /\/\*\s*ExcludeStart\s*\*\/[\w\W]*?\/\*\s*ExcludeEnd\s*\*\//ig, | ||
| - "" | ||
| - ) + wrapper[ 1 ] | ||
| + start: wrapper[ 0 ].replace( /\/\*\s*eslint(?: |-).*\s*\*\/\n/, "" ), | ||
| + end: wrapper[ 1 ] | ||
| }, | ||
| rawText: {}, | ||
| onBuildWrite: convert | ||
View
6
dist/.eslintrc → dist/.eslintrc.json
| @@ -1,10 +1,12 @@ | ||
| { | ||
| - "extends": "../src/.eslintrc", | ||
| + "extends": "../src/.eslintrc.json", | ||
| "rules": { | ||
| - // That is okay for built version | ||
| + // That is okay for the built version | ||
| "no-multiple-empty-lines": "off", | ||
| // Because sizze is not compatible to jquery code style | ||
| + "no-nested-ternary": "off", | ||
| + "no-unused-expressions": "off", | ||
| "lines-around-comment": "off", | ||
| "space-in-parens": "off", | ||
| "camelcase": "off", | ||
Oops, something went wrong.
Showing you all comments on commits in this comparison.
|
This was PR #3225, I forgot to link the commit. |
|
This was PR #3225, I forgot to link the commit. |
|
@markelog Why the PR reference in the commit title? It shoulnd't be there, instead there should be the line saying |
|
That's what happens when pr closed via github interface, we discuss this on of the meetings a month ago |
|
I missed the discussion; do you have a link? Was the decision that we accept landing commits in this way? |
No, there should be something in the archives though or maybe it was on github, this is the reason on why we allow only |
christianhaller
commented on 522f546
Aug 23, 2016
|
Makes my linter happy :) |
LaurentBarbareau
commented on e4fd41f
Nov 30, 2016
|
Just some remarks : It's good to transform a spaghetti code into a more clearer code but multiplying return statements is also considered as bad pratice because you can't figure out rapidly where you exit the function and where you continue with the other test conditions. Do what you want with this : I add that getData is quite a too generic name... |
As is the use of passive voice, since it obscures the actor. For an opposing point of view, I think early returns are much clearer than else-if chains because they allow you to clearly assert that specific cases are not in the domain of inputs at that point in the function. In any case, we rarely refactor code for style reasons because of the risk of introducing errors. jQuery is too widely used to go rearranging things for no functional reason. |
LaurentBarbareau
commented on e4fd41f
Nov 30, 2016
•
Edited 1 time
-
LaurentBarbareau
Nov 30, 2016
Edited 1 time
-
Nov 30, 2016
"The actor" is not me but the developer community. Was it on what you wanted me to be clearer about?
Yep ! That is exactly what I said to myself when I understood the transformation. Funny, no? :D |
|
@LaurentBarbareau We mainly did this transformation to satisfy eslint's recommended rules. There are multiple ways we could have done it, but we went with this one. Further transformation would just be a matter of taste. |
LaurentBarbareau
commented on e4fd41f
Nov 30, 2016
|
@timmywil Ok! It's a good argument. |
leinue
commented on 3bbcce6
Dec 2, 2016
|
this has a bug |
|
If you think so, file a bug with a test case and we can work it out. |