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.

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

Update to jQuery 3.x. #772

Merged
merged 2 commits into from
Feb 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"istanbul-instrumenter-loader": "^0.2.0",
"jaguarjs-jsdoc": "^1.0.2",
"jasmine-core": "^2.4.1",
"jquery": "~2.2.1",
"jquery": "^3.3",
"jsdoc": "^3.5",
"jsdoc-autoprivate": "0.0.1",
"json-loader": "^0.5.4",
Expand Down
4 changes: 2 additions & 2 deletions src/fetchQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ module.exports = (function () {
}
var wait = $.Deferred();
var process = $.Deferred();
wait.then(function () {
wait.done(function () {
$.when(callback.call(defer)).always(process.resolve);
}, process.resolve);
}).fail(process.resolve);
defer.__fetchQueue = wait;
this._addToQueue(defer, atEnd);
$.when(wait, process).always(function () {
Expand Down
4 changes: 2 additions & 2 deletions src/imageTile.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ module.exports = (function () {
this._image.src = this._url;

// attach a promise interface to `this`
defer.then(function () {
defer.done(function () {
this._fetched = true;
}.bind(this)).promise(this);
}
Expand All @@ -96,7 +96,7 @@ module.exports = (function () {
this.fadeIn = function (duration) {
var promise = this.fetch(), defer = $.Deferred();
$(this._image).css('display', 'none');
promise.then(function () {
promise.done(function () {
$(this._image).fadeIn(duration, function () {
defer.resolve();
});
Expand Down
6 changes: 5 additions & 1 deletion src/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ var object = function () {
}
}
m_promiseCount += 1;
promise.then(onDone, onDone);
if (promise.always) {
promise.always(onDone);
} else {
promise.then(onDone, onDone);
}
return m_this;
};

Expand Down
8 changes: 6 additions & 2 deletions src/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ module.exports = (function () {
*/
this.fetch = function () {
if (!this._fetched) {
$.get(this._url).then(function () {
$.get(this._url).done(function () {
this._fetched = true;
}.bind(this)).promise(this);
}
Expand Down Expand Up @@ -107,7 +107,11 @@ module.exports = (function () {
this.fetch();
}
// Call then on the new promise
this.then(onSuccess, onFailure);
if (this.done && this.fail) {
this.done(onSuccess).fail(onFailure);
} else {
this.then(onSuccess, onFailure);
}
return this;
};

Expand Down
2 changes: 1 addition & 1 deletion src/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ transform.lookup = function (projection) {

return $.ajax({
url: 'http://epsg.io/?q=' + code + '&format=json'
}).then(function (data) {
}).done(function (data) {
var result = (data.results || [])[0];
if (!result || !result.proj4) {
return defer.reject(data).promise();
Expand Down
103 changes: 65 additions & 38 deletions tests/cases/fetchQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ describe('geo.core.fetchQueue', function () {
reference += 1;
item.process = function () {
item.defer = $.Deferred();
item.defer.then(function () {
item.defer.done(function () {
report.push({ref: item.ref, success: true});
}, function () {
}).fail(function () {
report.push({ref: item.ref, success: false});
}).promise(item);
return item;
Expand All @@ -30,7 +30,7 @@ describe('geo.core.fetchQueue', function () {
reference = 1;
});

it('queue is the exepected size', function () {
it('queue is the expected size', function (done) {
var q = geo.fetchQueue({size: 2}), dlist = [];

expect(q.length).toBe(0);
Expand Down Expand Up @@ -70,15 +70,21 @@ describe('geo.core.fetchQueue', function () {
expect(q.processing).toBe(2);

dlist[0].defer.resolve();
expect(q.length).toBe(0);
expect(q.processing).toBe(2);

dlist[3].defer.resolve();
expect(q.length).toBe(0);
expect(q.processing).toBe(1);
window.setTimeout(function () { // wait for next time slice
expect(q.length).toBe(0);
expect(q.processing).toBe(2);

dlist[3].defer.resolve();
window.setTimeout(function () { // wait for next time slice
expect(q.length).toBe(0);
expect(q.processing).toBe(1);

done();
}, 0);
}, 0);
});

it('queue size can be changed', function () {
it('queue size can be changed', function (done) {
var q = geo.fetchQueue({size: 2}), dlist = [];

expect(q.size).toBe(2);
Expand All @@ -104,19 +110,26 @@ describe('geo.core.fetchQueue', function () {
expect(q.processing).toBe(3);

dlist[0].defer.resolve();
expect(q.length).toBe(2);
expect(q.processing).toBe(2);

dlist[1].defer.resolve();
expect(q.length).toBe(2);
expect(q.processing).toBe(1);

dlist[2].defer.resolve();
expect(q.length).toBe(1);
expect(q.processing).toBe(1);
window.setTimeout(function () { // wait for next time slice
expect(q.length).toBe(2);
expect(q.processing).toBe(2);

dlist[1].defer.resolve();
window.setTimeout(function () { // wait for next time slice
expect(q.length).toBe(2);
expect(q.processing).toBe(1);

dlist[2].defer.resolve();
window.setTimeout(function () { // wait for next time slice
expect(q.length).toBe(1);
expect(q.processing).toBe(1);
done();
}, 0);
}, 0);
}, 0);
});

it('queue removes and skips unneeded items', function () {
it('queue removes and skips unneeded items', function (done) {
var q = geo.fetchQueue({
size: 2,
track: 4,
Expand Down Expand Up @@ -155,21 +168,28 @@ describe('geo.core.fetchQueue', function () {
}
}

while (q.processing) {
for (i = 0; i < dlist.length; i += 1) {
if (dlist[i].defer) {
dlist[i].defer.resolve();
function process() {
if (q.processing) {
for (i = 0; i < dlist.length; i += 1) {
if (dlist[i].defer) {
dlist[i].defer.resolve();
}
}
window.setTimeout(process, 0);
return;
}
}

expect(report.length).toBe(reportOrder.length);
for (i = 0; i < report.length; i += 1) {
expect(report[i].ref).toBe(reportOrder[i]);
expect(report.length).toBe(reportOrder.length);
for (i = 0; i < report.length; i += 1) {
expect(report[i].ref).toBe(reportOrder[i]);
}
done();
}

process();
});

it('batch ordering', function () {
it('batch ordering', function (done) {
var q = geo.fetchQueue({size: 1}), dlist = [], i;
var reportOrder = [
1, 22, 23, 20, 19, 17, 13, 14, 16, 11, 10, 5,
Expand All @@ -193,18 +213,25 @@ describe('geo.core.fetchQueue', function () {
q.add(dlist[i], dlist[i].process, (i % 3) === 2);
}

while (q.processing) {
for (i = 0; i < dlist.length; i += 1) {
if (dlist[i].defer) {
dlist[i].defer.resolve();
function process() {
if (q.processing) {
for (i = 0; i < dlist.length; i += 1) {
if (dlist[i].defer) {
dlist[i].defer.resolve();
}
}
window.setTimeout(process, 0);
return;
}
}

expect(report.length).toBe(reportOrder.length);
for (i = 0; i < report.length; i += 1) {
expect(report[i].ref).toBe(reportOrder[i]);
expect(report.length).toBe(reportOrder.length);
for (i = 0; i < report.length; i += 1) {
expect(report[i].ref).toBe(reportOrder[i]);
}
done();
}

process();
});
});
});
9 changes: 6 additions & 3 deletions tests/cases/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('geo.tile', function () {
});
});

it('failure', function () {
it('failure', function (done) {
// create a mocked sinon server instance that always responds with 404.
var server = sinon.fakeServer.create();

Expand All @@ -165,8 +165,11 @@ describe('geo.tile', function () {
});

server.respond();
expect(called).toBe(true);
server.restore();
window.setTimeout(function () { // wait for the next time slice
expect(called).toBe(true);
server.restore();
done();
}, 0);
});
});

Expand Down
15 changes: 9 additions & 6 deletions tests/cases/tileLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,7 @@ describe('geo.tileLayer', function () {
expect(Object.keys(l.activeTiles).length).toBe(0);
});

it('invalid tile url', function () {
it('invalid tile url', function (done) {
var server = sinon.fakeServer.create();
var spy = sinon.spy();
sinon.stub(console, 'warn', function () {});
Expand All @@ -1483,11 +1483,14 @@ describe('geo.tileLayer', function () {
t.catch(spy);

server.respond();
expect(console.warn.calledOnce);
expect(spy.calledOnce).toBe(true);
expect(l.canvas().find('.geo-tile-container').length).toBe(0);
server.restore();
console.warn.restore();
window.setTimeout(function () { // wait for the next time slice
expect(console.warn.calledOnce);
expect(spy.calledOnce).toBe(true);
expect(l.canvas().find('.geo-tile-container').length).toBe(0);
server.restore();
console.warn.restore();
done();
}, 0);
});

it('cropped tile', function () {
Expand Down
24 changes: 15 additions & 9 deletions tests/cases/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe('geo.transform', function () {
expect(p.forward({x: 10, y: -10, z: 0})).toEqual({x: 10, y: -10, z: 0});
});

it('lookup', function () {
it('lookup', function (done) {
var spy = sinon.spy(), request;
geo.transform.lookup('EPSG:5000').then(spy);

Expand Down Expand Up @@ -147,16 +147,19 @@ describe('geo.transform', function () {
}]
}));

expect(spy.calledOnce).toBe(true);
expect(geo.transform.defs.hasOwnProperty('EPSG:5000')).toBe(true);
window.setTimeout(function () { // wait for the next time slice
expect(spy.calledOnce).toBe(true);
expect(geo.transform.defs.hasOwnProperty('EPSG:5000')).toBe(true);

geo.transform.lookup('EPSG:5000');
expect(server.requests.length).toBe(1);
geo.transform.lookup('EPSG:5000');
expect(server.requests.length).toBe(1);
done();
}, 0);
});

it('invalid projection code', function () {
it('invalid projection code', function (done) {
var spy = sinon.spy(), request;
geo.transform.lookup('EPSG:5001').fail(spy);
geo.transform.lookup('EPSG:5001').then(spy);

request = server.requests[0];
request.respond(200, {'Content-Type': 'application/json'}, JSON.stringify({
Expand All @@ -165,8 +168,11 @@ describe('geo.transform', function () {
results: []
}));

expect(spy.calledOnce).toBe(true);
expect(geo.transform.defs.hasOwnProperty('EPSG:5001')).toBe(false);
window.setTimeout(function () { // wait for the next time slice
Copy link
Member

Choose a reason for hiding this comment

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

why we need window timeout now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jQuery 3 is always asynchronous for certain requests where jQuery 2 was sometimes synchronous. Specifically, we are making an ajax request to get the transform from a web service. We mock the web service for the test, so the mock returns immediately. But, in jQuery 3, the callback is fired in the next time slice rather than synchronously, so we wait 0 ms (just to be in the next time slice) to make sure the callback has been triggered.

In jQuery 2, the behavior was ajax->mock response->callback->next time slice.

In jQuery 3, the behavior is ajax->mock response->next time slice->callback.

If we were doing an actual request which couldn't be served from browser cache, we would have to wait an unknown amount of time before the callback would be triggered. It is only because we are mocking the response that the immediate check worked in jQuery 2 (and the check with a 0-duration wait works in jQuery 3). Rather than use window.setTimeout, we could have used time mocking, but that doesn't make the test any clearer in my opinion.

Copy link
Member

Choose a reason for hiding this comment

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

thanks for the detailed explanation @manthey.

expect(spy.calledOnce).toBe(true);
expect(geo.transform.defs.hasOwnProperty('EPSG:5001')).toBe(false);
done();
}, 0);
});

it('unknown projection type', function () {
Expand Down
3 changes: 3 additions & 0 deletions tests/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ module.exports.mockDate = function (delta) {
startDate += mockDelta;
return new origDate(startDate);
};
geo.mockedDate.now = function () {
return startDate;
};

window.Date = geo.mockedDate;
};
Expand Down
2 changes: 1 addition & 1 deletion tutorials/video_on_map/index.pug
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ block mainTutorial
:markdown-it
The video requires a feature layer that supports video quads.

+codeblock('javascript', 2).
+codeblock('javascript', 2)(htmlvideo=true).
var layer = map.createLayer('feature', {
features: ['quad.video']
});
Expand Down