From 9f8cb3a96956df8760a559180031417bc890b014 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Wed, 13 Sep 2017 08:56:49 -0400 Subject: [PATCH 1/2] Use can-globals/location for setting the location can-globals uses caching so any time can-route looks for the location object, it gets a cached version. This adds functionality to set the location on each async task, so that it's always correct for the given zone. Fixes #338 --- lib/startup.js | 3 ++- lib/zones/globals.js | 12 ++++++++++-- test/async_test.js | 28 +++++++++++++++------------- test/tests/plain/home/home.js | 2 +- test/tests/plain/orders/orders.js | 2 +- test/timeout_test.js | 17 +++++++++-------- 6 files changed, 38 insertions(+), 26 deletions(-) diff --git a/lib/startup.js b/lib/startup.js index cb71f0b..b2de661 100644 --- a/lib/startup.js +++ b/lib/startup.js @@ -1,7 +1,8 @@ exports.modules = { "can": "can-util/namespace", "DOCUMENT": "can-util/dom/document/document", - "domMutate": "can-util/dom/mutate/mutate" + "domMutate": "can-util/dom/mutate/mutate", + "LOCATION": "can-globals/location/location" }; // Logic that needs to run when steal starts up. diff --git a/lib/zones/globals.js b/lib/zones/globals.js index c4baa65..604c7aa 100644 --- a/lib/zones/globals.js +++ b/lib/zones/globals.js @@ -1,7 +1,11 @@ var url = require("url"); +var noop = Function.prototype; module.exports = function(document, stream, steal, modules){ - var DOCUMENT = modules.DOCUMENT || function(){}; + // canjs stuff, this should be moved (I think) + var DOCUMENT = modules.DOCUMENT || noop; + var LOCATION = modules.LOCATION || noop; + var loader = steal.loader; var request = stream.request; var pushResources = stream[Symbol.for("donessr.incremental")]; @@ -10,7 +14,7 @@ module.exports = function(document, stream, steal, modules){ var location = url.parse(request.url, true); document.location = location; - var canDocument, language, + var canDocument, canLocation, language, getHeader = function(name) { if(request.headers) { return request.headers[name]; @@ -36,6 +40,9 @@ module.exports = function(document, stream, steal, modules){ canDocument = DOCUMENT(); DOCUMENT(document); + canLocation = LOCATION(); + LOCATION(location); + language = navigator.language; var lang = getHeader("accept-language"); if(lang) { @@ -44,6 +51,7 @@ module.exports = function(document, stream, steal, modules){ }, afterTask: function(){ DOCUMENT(canDocument); + LOCATION(canLocation); navigator.language = language; } }; diff --git a/test/async_test.js b/test/async_test.js index 8ab66b2..b386a4d 100644 --- a/test/async_test.js +++ b/test/async_test.js @@ -24,25 +24,27 @@ describe("async rendering", function(){ it("basics works", function(done){ this.render("/").pipe(through(function(buffer){ - var html = buffer.toString(); - var node = helpers.dom(html); + Promise.resolve().then(function(){ + var html = buffer.toString(); + var node = helpers.dom(html); - var message = node.getElementById("orders"); + var message = node.getElementById("orders"); - assert(message, "Found the message element that was rendered" + - "asynchronously"); + assert(message, "Found the message element that was rendered" + + "asynchronously"); - var cache = helpers.getXhrCache(node); + var cache = helpers.getXhrCache(node); - assert.equal(cache.length, 1, "There is one item in cache"); - assert.equal(cache[0].request.url, "foo://bar", "correct request url"); + assert.equal(cache.length, 1, "There is one item in cache"); + assert.equal(cache[0].request.url, "foo://bar", "correct request url"); - var resp = cache[0].response; + var resp = cache[0].response; - assert.equal(resp.headers, "Content-Type: application/json", - "Header was added"); - done(); + assert.equal(resp.headers, "Content-Type: application/json", + "Header was added"); + }) + .then(done, done); })); }); @@ -50,7 +52,7 @@ describe("async rendering", function(){ var request = { url: "/", headers: { - "accept-language": "en-US" + "accept-language": "en-US" } }; this.render(request).pipe(through(function(buffer){ diff --git a/test/tests/plain/home/home.js b/test/tests/plain/home/home.js index 5cab91d..3b30373 100644 --- a/test/tests/plain/home/home.js +++ b/test/tests/plain/home/home.js @@ -8,6 +8,6 @@ var ViewModel = Map.extend({ Component.extend({ tag: "home-page", view: view, - viewModel: ViewModel, + ViewModel: ViewModel, leakScope: true }); diff --git a/test/tests/plain/orders/orders.js b/test/tests/plain/orders/orders.js index 075018e..185406f 100644 --- a/test/tests/plain/orders/orders.js +++ b/test/tests/plain/orders/orders.js @@ -25,5 +25,5 @@ var ViewModel = Map.extend({ Component.extend({ tag: "order-history", view: view, - viewModel: ViewModel + ViewModel: ViewModel }); diff --git a/test/timeout_test.js b/test/timeout_test.js index 3f18f39..21adfe3 100644 --- a/test/timeout_test.js +++ b/test/timeout_test.js @@ -31,16 +31,17 @@ describe("Timeouts", function(){ it("Doesn't timeout if rendered quickly enough", function(done){ this.render("/fast").pipe(through(function(buffer){ - var html = buffer.toString(); - var node = helpers.dom(html); + Promise.resolve().then(function(){ + var html = buffer.toString(); + var node = helpers.dom(html); - var result = node.getElementById("result").innerHTML; + var result = node.getElementById("result").innerHTML; + assert.equal(result, "passed", "Timed out"); - assert.equal(result, "passed", "Timed out"); - - var debug = node.getElementById("done-ssr-debug"); - assert.ok(!debug, "debug node not present"); - done(); + var debug = node.getElementById("done-ssr-debug"); + assert.ok(!debug, "debug node not present"); + }) + .then(done, done); })); }); From 330a4a77f77f8da973b621bef2033f1711636803 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Mon, 18 Sep 2017 15:24:27 -0400 Subject: [PATCH 2/2] Call route._teardown() --- lib/strategies/incremental/render_app.js | 2 ++ lib/strategies/incremental/styles_zone.js | 7 +++---- lib/zones/route_data.js | 4 ++++ perf/make_leak.js | 9 +++------ test/incremental_test.js | 2 +- test/leak_test.js | 3 ++- test/tests/async/orders/orders.js | 2 +- 7 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/strategies/incremental/render_app.js b/lib/strategies/incremental/render_app.js index 5082ac0..df42cff 100644 --- a/lib/strategies/incremental/render_app.js +++ b/lib/strategies/incremental/render_app.js @@ -69,6 +69,8 @@ module.exports = function(stream, request, modules, context, plugins){ } }); + zone.data.runCalled = true; + return { zone: zone, promise: promise, diff --git a/lib/strategies/incremental/styles_zone.js b/lib/strategies/incremental/styles_zone.js index 2ee2238..09ca3c9 100644 --- a/lib/strategies/incremental/styles_zone.js +++ b/lib/strategies/incremental/styles_zone.js @@ -3,7 +3,7 @@ var canData = require("can-util/dom/data/data"); module.exports = function(doc, bundleHelpers, can){ return function(data){ - var promises = [], initialRun = false, + var promises = [], usesCan = typeof can !== "undefined"; // Keep track of can-import calls that happen during the initial render @@ -28,14 +28,13 @@ module.exports = function(doc, bundleHelpers, can){ plugins: [assetsZone(doc, bundleHelpers, can)], beforeTask: function(){ - if(!initialRun && usesCan) { + if(!data.runCalled && usesCan) { oldCanImport = can.view.callbacks._tags["can-import"]; can.view.callbacks._tags["can-import"] = canImport; } }, afterTask: function(){ - if(!initialRun) { - initialRun = true; + if(!data.runCalled) { if(usesCan) { can.view.callbacks._tags["can-import"] = oldCanImport; } diff --git a/lib/zones/route_data.js b/lib/zones/route_data.js index 8a97905..474fbe9 100644 --- a/lib/zones/route_data.js +++ b/lib/zones/route_data.js @@ -69,6 +69,10 @@ module.exports = function(can){ data.statusCode = extractStatusCode(viewModel); data.state = viewModel; viewModel.removeEventListener('statusCode', noop); + + if(hasCanRoute()) { + can.route._teardown(); + } } } }; diff --git a/perf/make_leak.js b/perf/make_leak.js index 5c24a02..0e352a6 100644 --- a/perf/make_leak.js +++ b/perf/make_leak.js @@ -27,13 +27,10 @@ var i = 100; next(); function next() { - if(i !== 100) { - //console.log(Object.keys(System._loader.modules)); - var len = Object.keys(System._loader.modules['can-util@3.9.6#dom/data/core'].module.default._data).length; - console.log("Keys:",len); - } - if(i === 0) return; + if(i === 0) { + return; + } i--; renderThen("/").then(next); diff --git a/test/incremental_test.js b/test/incremental_test.js index 518b122..c01a1f7 100644 --- a/test/incremental_test.js +++ b/test/incremental_test.js @@ -49,7 +49,7 @@ describe("Incremental rendering", function(){ it("Sends the correct rendering instructions", function(){ var instr = this.result.instructions[0][1]; assert.equal(instr.route, "0.2.7"); - + // Easier to test var nodeAsJson = JSON.stringify(instr.node); assert.ok(/ORDER-HISTORY/.test(nodeAsJson), "adds the order-history component"); diff --git a/test/leak_test.js b/test/leak_test.js index c356e1e..c02d04c 100644 --- a/test/leak_test.js +++ b/test/leak_test.js @@ -21,7 +21,7 @@ describe("Memory leaks", function(){ this.render = function(pth){ return new Promise(function(resolve, reject){ var stream = through(function(buffer){ - resolve(buffer); + setTimeout(resolve, 300); }); stream.on("error", reject); render(pth).pipe(stream); @@ -37,6 +37,7 @@ describe("Memory leaks", function(){ }); it("do not happen", function(done){ + iterate(10, () => { return this.render("/"); }) diff --git a/test/tests/async/orders/orders.js b/test/tests/async/orders/orders.js index fb12c69..4fddf77 100644 --- a/test/tests/async/orders/orders.js +++ b/test/tests/async/orders/orders.js @@ -31,5 +31,5 @@ var ViewModel = Map.extend({ Component.extend({ tag: "order-history", view: view, - viewModel: ViewModel + ViewModel: ViewModel });