diff --git a/History.md b/History.md index 472159a56f..ef07249e1f 100644 --- a/History.md +++ b/History.md @@ -20,6 +20,15 @@ * Add `frame-src` to `browser-policy-content` and account for cross-browser CSP disparities. +* Transform functions must return objects and may not change the `_id` field + (though they may leave it out) + +* Upgrade jQuery from 1.8.2 to 1.10.2. + XXX see http://jquery.com/upgrade-guide/1.9/ for incompatibilities + XXX consider taking 1.11 instead, which was released this week + +* `force-ssl`: don't require SSL during `meteor run` in IPv6 environments. + ## v0.7.0.1 diff --git a/docs/client/api.html b/docs/client/api.html index 8b5a00e1b7..e26d875cf6 100644 --- a/docs/client/api.html +++ b/docs/client/api.html @@ -654,7 +654,8 @@ methods, documents are passed through the `transform` function before being returned or passed to callbacks. This allows you to add methods or otherwise modify the contents of your collection from their database representation. You can also specify `transform` on a particular `find`, `findOne`, `allow`, or -`deny` call. +`deny` call. Transform functions must return an object and they may not change +the value of the document's `_id` field (though it's OK to leave it out). // An Animal class that takes a document in its constructor Animal = function (doc) { diff --git a/meteor b/meteor index 95a9676e78..3b6fe46738 100755 --- a/meteor +++ b/meteor @@ -1,6 +1,8 @@ #!/bin/bash -BUNDLE_VERSION=0.3.28 +# danger will robinson! mother:config/download-dev-bundles.sh only goes up to +# 0.3.30! +BUNDLE_VERSION=0.3.29 # OS Check. Put here because here is where we download the precompiled # bundles that are arch specific. diff --git a/packages/force-ssl/force_ssl_server.js b/packages/force-ssl/force_ssl_server.js index 36d9418acc..c9306f6774 100644 --- a/packages/force-ssl/force_ssl_server.js +++ b/packages/force-ssl/force_ssl_server.js @@ -22,11 +22,12 @@ httpServer.addListener('request', function (req, res) { // Determine if the connection is only over localhost. Both we // received it on localhost, and all proxies involved received on // localhost. + var localhostRegexp = /^\s*(127\.0\.0\.1|::1)\s*$/; var isLocal = ( - remoteAddress === "127.0.0.1" && + localhostRegexp.test(remoteAddress) && (!req.headers['x-forwarded-for'] || _.all(req.headers['x-forwarded-for'].split(','), function (x) { - return /\s*127\.0\.0\.1\s*/.test(x); + return localhostRegexp.test(x); }))); // Determine if the connection was over SSL at any point. Either we diff --git a/packages/meteor/fiber_stubs_client.js b/packages/meteor/fiber_stubs_client.js index 3babd0e08f..46163a26e1 100644 --- a/packages/meteor/fiber_stubs_client.js +++ b/packages/meteor/fiber_stubs_client.js @@ -6,71 +6,3 @@ Meteor._noYieldsAllowed = function (f) { return f(); }; - -// An even simpler queue of tasks than the fiber-enabled one. This one just -// runs all the tasks when you call runTask or flush, synchronously. -// -Meteor._SynchronousQueue = function () { - var self = this; - self._tasks = []; - self._running = false; -}; - -_.extend(Meteor._SynchronousQueue.prototype, { - runTask: function (task) { - var self = this; - if (!self.safeToRunTask()) - throw new Error("Could not synchronously run a task from a running task"); - self._tasks.push(task); - var tasks = self._tasks; - self._tasks = []; - self._running = true; - try { - while (!_.isEmpty(tasks)) { - var t = tasks.shift(); - try { - t(); - } catch (e) { - if (_.isEmpty(tasks)) { - // this was the last task, that is, the one we're calling runTask - // for. - throw e; - } else { - Meteor._debug("Exception in queued task: " + e.stack); - } - } - } - } finally { - self._running = false; - } - }, - - queueTask: function (task) { - var self = this; - var wasEmpty = _.isEmpty(self._tasks); - self._tasks.push(task); - // Intentionally not using Meteor.setTimeout, because it doesn't like runing - // in stubs for now. - if (wasEmpty) - setTimeout(_.bind(self.flush, self), 0); - }, - - flush: function () { - var self = this; - self.runTask(function () {}); - }, - - drain: function () { - var self = this; - if (!self.safeToRunTask()) - return; - while (!_.isEmpty(self._tasks)) { - self.flush(); - } - }, - - safeToRunTask: function () { - var self = this; - return !self._running; - } -}); diff --git a/packages/meteor/package.js b/packages/meteor/package.js index 439d80e6f2..9e2db6e68e 100644 --- a/packages/meteor/package.js +++ b/packages/meteor/package.js @@ -23,6 +23,7 @@ Package.on_use(function (api) { api.add_files('errors.js', ['client', 'server']); api.add_files('fiber_helpers.js', 'server'); api.add_files('fiber_stubs_client.js', 'client'); + api.add_files('unyielding_queue.js'); api.add_files('startup_client.js', ['client']); api.add_files('startup_server.js', ['server']); api.add_files('debug.js', ['client', 'server']); diff --git a/packages/meteor/unyielding_queue.js b/packages/meteor/unyielding_queue.js new file mode 100644 index 0000000000..95c62b23a4 --- /dev/null +++ b/packages/meteor/unyielding_queue.js @@ -0,0 +1,72 @@ +// A simpler version of Meteor._SynchronousQueue with the same external +// interface. It runs on both client and server, unlike _SynchronousQueue which +// only runs on the server. When used on the server, tasks may not yield. This +// one just runs all the tasks when you call runTask or flush, synchronously. +// It itself also does not yield. +// +Meteor._UnyieldingQueue = function () { + var self = this; + self._tasks = []; + self._running = false; +}; + +_.extend(Meteor._UnyieldingQueue.prototype, { + runTask: function (task) { + var self = this; + if (!self.safeToRunTask()) + throw new Error("Could not synchronously run a task from a running task"); + self._tasks.push(task); + var tasks = self._tasks; + self._tasks = []; + self._running = true; + try { + while (!_.isEmpty(tasks)) { + var t = tasks.shift(); + try { + Meteor._noYieldsAllowed(function () { + t(); + }); + } catch (e) { + if (_.isEmpty(tasks)) { + // this was the last task, that is, the one we're calling runTask + // for. + throw e; + } else { + Meteor._debug("Exception in queued task: " + e.stack); + } + } + } + } finally { + self._running = false; + } + }, + + queueTask: function (task) { + var self = this; + var wasEmpty = _.isEmpty(self._tasks); + self._tasks.push(task); + // Intentionally not using Meteor.setTimeout, because it doesn't like runing + // in stubs for now. + if (wasEmpty) + setTimeout(_.bind(self.flush, self), 0); + }, + + flush: function () { + var self = this; + self.runTask(function () {}); + }, + + drain: function () { + var self = this; + if (!self.safeToRunTask()) + return; + while (!_.isEmpty(self._tasks)) { + self.flush(); + } + }, + + safeToRunTask: function () { + var self = this; + return !self._running; + } +}); diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 67c184aca5..71977c86bf 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -7,13 +7,34 @@ // ObserveHandle: the return value of a live query. -LocalCollection = function (name) { +LocalCollection = function (options) { var self = this; - self.name = name; + options = options || {}; + + self.name = options.name; // _id -> document (also containing id) self._docs = new LocalCollection._IdMap; - self._observeQueue = new Meteor._SynchronousQueue(); + // When writing to this collection, we batch all observeChanges callbacks + // until the end of the write, and run them at this point. On the server, we + // use a single SynchronousQueue to do so, so that we never deliver callbacks + // out of order even if other writes occur during a yield. On the client, or + // on the server if we promise that our callbacks will never yield via an + // undocumented option, we use the simpler UnyieldingQueue. + // + // (What is the _observeCallbacksWillNeverYield option for? In some cases, it + // can be nice (on the server) to be able to write to a LocalCollection + // without yielding (eg, in a _noYieldsAllowed block). It's necessary to + // provide non-yielding allow callbacks in that case, but just doing that + // wouldn't be good enough if we always used SynchronousQueue on the server, + // since it tends to yield in order to run even non-yielding callbacks.) + var queueClass; + if (Meteor._SynchronousQueue && !options._observeCallbacksWillNeverYield) { + queueClass = Meteor._SynchronousQueue; + } else { + queueClass = Meteor._UnyieldingQueue; + } + self._observeQueue = new queueClass(); self.next_qid = 1; // live query id generator @@ -21,13 +42,13 @@ LocalCollection = function (name) { // ordered: bool. ordered queries have addedBefore/movedBefore callbacks. // results: array (ordered) or object (unordered) of current results // (aliased with self._docs!) - // results_snapshot: snapshot of results. null if not paused. + // resultsSnapshot: snapshot of results. null if not paused. // cursor: Cursor object for the query. // selector, sorter, (callbacks): functions self.queries = {}; - // null if not saving originals; an IdMap from id to original document value if - // saving originals. See comments before saveOriginals(). + // null if not saving originals; an IdMap from id to original document value + // if saving originals. See comments before saveOriginals(). self._savedOriginals = null; // True when observers are paused and we should not send callbacks. @@ -107,12 +128,9 @@ LocalCollection.Cursor = function (collection, selector, options) { self.fields = options.fields; if (self.fields) - self.projection_f = LocalCollection._compileProjection(self.fields); + self.projectionFn = LocalCollection._compileProjection(self.fields); - if (options.transform && typeof Deps !== "undefined") - self._transform = Deps._makeNonreactive(LocalCollection.wrapTransform(options.transform)); - else - self._transform = LocalCollection.wrapTransform(options.transform); + self._transform = LocalCollection.wrapTransform(options.transform); // db_objects is an array of the objects that match the cursor. (It's always // an array, never an IdMap: LocalCollection.Cursor is always ordered.) @@ -152,7 +170,7 @@ LocalCollection.Cursor.prototype.forEach = function (callback, thisArg) { var self = this; if (self.db_objects === null) - self.db_objects = self._getRawObjects(true); + self.db_objects = self._getRawObjects({ordered: true}); if (self.reactive) self._depend({ @@ -163,8 +181,8 @@ LocalCollection.Cursor.prototype.forEach = function (callback, thisArg) { while (self.cursor_pos < self.db_objects.length) { var elt = EJSON.clone(self.db_objects[self.cursor_pos]); - if (self.projection_f) - elt = self.projection_f(elt); + if (self.projectionFn) + elt = self.projectionFn(elt); if (self._transform) elt = self._transform(elt); callback.call(thisArg, elt, self.cursor_pos, self); @@ -202,7 +220,7 @@ LocalCollection.Cursor.prototype.count = function () { true /* allow the observe to be unordered */); if (self.db_objects === null) - self.db_objects = self._getRawObjects(true); + self.db_objects = self._getRawObjects({ordered: true}); return self.db_objects.length; }; @@ -284,12 +302,10 @@ _.extend(LocalCollection.Cursor.prototype, { sorter: ordered && self.sorter, distances: ( self.matcher.hasGeoQuery() && ordered && new LocalCollection._IdMap), - results_snapshot: null, + resultsSnapshot: null, ordered: ordered, cursor: self, - observeChanges: options.observeChanges, - fields: self.fields, - projection_f: self.projection_f + projectionFn: self.projectionFn }; var qid; @@ -299,9 +315,10 @@ _.extend(LocalCollection.Cursor.prototype, { qid = self.collection.next_qid++; self.collection.queries[qid] = query; } - query.results = self._getRawObjects(ordered, query.distances); + query.results = self._getRawObjects({ + ordered: ordered, distances: query.distances}); if (self.collection.paused) - query.results_snapshot = (ordered ? [] : new LocalCollection._IdMap); + query.resultsSnapshot = (ordered ? [] : new LocalCollection._IdMap); // wrap callbacks we were passed. callbacks only fire when not paused and // are never undefined @@ -317,17 +334,18 @@ _.extend(LocalCollection.Cursor.prototype, { var context = this; var args = arguments; - if (fieldsIndex !== undefined && self.projection_f) { - args[fieldsIndex] = self.projection_f(args[fieldsIndex]); + if (self.collection.paused) + return; + + if (fieldsIndex !== undefined && self.projectionFn) { + args[fieldsIndex] = self.projectionFn(args[fieldsIndex]); if (ignoreEmptyFields && _.isEmpty(args[fieldsIndex])) return; } - if (!self.collection.paused) { - self.collection._observeQueue.queueTask(function () { - f.apply(context, args); - }); - } + self.collection._observeQueue.queueTask(function () { + f.apply(context, args); + }); }; }; query.added = wrapCallback(options.added, 1); @@ -395,13 +413,13 @@ _.extend(LocalCollection.Cursor.prototype, { // argument, this function will clear it and use it for this purpose (otherwise // it will just create its own _IdMap). The observeChanges implementation uses // this to remember the distances after this function returns. -LocalCollection.Cursor.prototype._getRawObjects = function (ordered, - distances) { +LocalCollection.Cursor.prototype._getRawObjects = function (options) { var self = this; + options = options || {}; // XXX use OrderedDict instead of array, and make IdMap and OrderedDict // compatible - var results = ordered ? [] : new LocalCollection._IdMap; + var results = options.ordered ? [] : new LocalCollection._IdMap; // fast path for single ID value if (self._selectorId !== undefined) { @@ -413,7 +431,7 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered, var selectedDoc = self.collection._docs.get(self._selectorId); if (selectedDoc) { - if (ordered) + if (options.ordered) results.push(selectedDoc); else results.set(self._selectorId, selectedDoc); @@ -426,17 +444,20 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered, // in the observeChanges case, distances is actually part of the "query" (ie, // live results set) object. in other cases, distances is only used inside // this function. - if (self.matcher.hasGeoQuery() && ordered) { - if (distances) + var distances; + if (self.matcher.hasGeoQuery() && options.ordered) { + if (options.distances) { + distances = options.distances; distances.clear(); - else + } else { distances = new LocalCollection._IdMap(); + } } self.collection._docs.forEach(function (doc, id) { var matchResult = self.matcher.documentMatches(doc); if (matchResult.result) { - if (ordered) { + if (options.ordered) { results.push(doc); if (distances && matchResult.distance !== undefined) distances.set(id, matchResult.distance); @@ -452,7 +473,7 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered, return true; // continue }); - if (!ordered) + if (!options.ordered) return results; if (self.sorter) { @@ -542,31 +563,60 @@ LocalCollection.prototype.insert = function (doc, callback) { return id; }; -LocalCollection.prototype.remove = function (selector, callback) { +// Iterates over a subset of documents that could match selector; calls +// f(doc, id) on each of them. Specifically, if selector specifies +// specific _id's, it only looks at those. doc is *not* cloned: it is the +// same object that is in _docs. +LocalCollection.prototype._eachPossiblyMatchingDoc = function (selector, f) { var self = this; - var remove = []; - - var queriesToRecompute = []; - var matcher = new Minimongo.Matcher(selector, self); - - // Avoid O(n) for "remove a single doc by ID". var specificIds = LocalCollection._idsMatchedBySelector(selector); if (specificIds) { - _.each(specificIds, function (id) { - // We still have to run matcher, in case it's something like - // {_id: "X", a: 42} + for (var i = 0; i < specificIds.length; ++i) { + var id = specificIds[i]; var doc = self._docs.get(id); - if (doc && matcher.documentMatches(doc).result) - remove.push(id); - }); + if (doc) { + var breakIfFalse = f(doc, id); + if (breakIfFalse === false) + break; + } + } } else { - self._docs.forEach(function (doc, id) { - if (matcher.documentMatches(doc).result) { - remove.push(id); + self._docs.forEach(f); + } +}; + +LocalCollection.prototype.remove = function (selector, callback) { + var self = this; + + // Easy special case: if we're not calling observeChanges callbacks and we're + // not saving originals and we got asked to remove everything, then just empty + // everything directly. + if (self.paused && !self._savedOriginals && EJSON.equals(selector, {})) { + var result = self._docs.size(); + self._docs.clear(); + _.each(self.queries, function (query) { + if (query.ordered) { + query.results = []; + } else { + query.results.clear(); } }); + if (callback) { + Meteor.defer(function () { + callback(null, result); + }); + } + return result; } + var matcher = new Minimongo.Matcher(selector, self); + var remove = []; + self._eachPossiblyMatchingDoc(selector, function (doc, id) { + if (matcher.documentMatches(doc).result) + remove.push(id); + }); + + var queriesToRecompute = []; var queryRemove = []; for (var i = 0; i < remove.length; i++) { var removeId = remove[i]; @@ -597,7 +647,7 @@ LocalCollection.prototype.remove = function (selector, callback) { LocalCollection._recomputeResults(query); }); self._observeQueue.drain(); - var result = remove.length; + result = remove.length; if (callback) Meteor.defer(function () { callback(null, result); @@ -620,7 +670,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { // Save the original results of any query that we might need to // _recomputeResults on, because _modifyAndNotify will mutate the objects in // it. (We don't need to save the original results of paused queries because - // they already have a results_snapshot and we won't be diffing in + // they already have a resultsSnapshot and we won't be diffing in // _recomputeResults.) var qidToOriginalResults = {}; _.each(self.queries, function (query, qid) { @@ -633,7 +683,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { var updateCount = 0; - self._docs.forEach(function (doc, id) { + self._eachPossiblyMatchingDoc(selector, function (doc, id) { var queryResult = matcher.documentMatches(doc); if (queryResult.result) { // XXX Should we save the original even if mod ends up being a no-op? @@ -842,7 +892,8 @@ LocalCollection._recomputeResults = function (query, oldResults) { oldResults = query.results; if (query.distances) query.distances.clear(); - query.results = query.cursor._getRawObjects(query.ordered, query.distances); + query.results = query.cursor._getRawObjects({ + ordered: query.ordered, distances: query.distances}); if (!query.paused) { LocalCollection._diffQueryChanges( @@ -938,7 +989,7 @@ LocalCollection.prototype.pauseObservers = function () { for (var qid in this.queries) { var query = this.queries[qid]; - query.results_snapshot = EJSON.clone(query.results); + query.resultsSnapshot = EJSON.clone(query.results); } }; @@ -961,8 +1012,8 @@ LocalCollection.prototype.resumeObservers = function () { // Diff the current results against the snapshot and send to observers. // pass the query object for its observer callbacks. LocalCollection._diffQueryChanges( - query.ordered, query.results_snapshot, query.results, query); - query.results_snapshot = null; + query.ordered, query.resultsSnapshot, query.results, query); + query.resultsSnapshot = null; } self._observeQueue.drain(); }; diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 68c6b0203d..4bb58d2b94 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2170,7 +2170,8 @@ Tinytest.add("minimongo - observe ordered", function (test) { handle.stop(); // test _suppress_initial - handle = c.find({}, {sort: {a: -1}}).observe(_.extend(cbs, {_suppress_initial: true})); + handle = c.find({}, {sort: {a: -1}}).observe(_.extend({ + _suppress_initial: true}, cbs)); test.equal(operations.shift(), undefined); c.insert({a:100}); test.equal(operations.shift(), ['added', {a:100}, 0, idA2]); @@ -2197,6 +2198,21 @@ Tinytest.add("minimongo - observe ordered", function (test) { test.equal(operations.shift(), ['changed', {a:3.5}, 0, {a:3}]); handle.stop(); + // test observe limit with pre-existing docs + c.remove({}); + c.insert({a: 1}); + c.insert({_id: 'two', a: 2}); + c.insert({a: 3}); + handle = c.find({}, {sort: {a: 1}, limit: 2}).observe(cbs); + test.equal(operations.shift(), ['added', {a:1}, 0, null]); + test.equal(operations.shift(), ['added', {a:2}, 1, null]); + test.equal(operations.shift(), undefined); + c.remove({a: 2}); + test.equal(operations.shift(), ['removed', 'two', 1, {a:2}]); + test.equal(operations.shift(), ['added', {a:3}, 1, null]); + test.equal(operations.shift(), undefined); + handle.stop(); + // test _no_indices c.remove({}); @@ -2565,6 +2581,14 @@ Tinytest.add("minimongo - pause", function (test) { test.equal(operations.shift(), ['changed', {a:3}, 0, {a:1}]); test.length(operations, 0); + // test special case for remove({}) + c.pauseObservers(); + test.equal(c.remove({}), 1); + test.length(operations, 0); + c.resumeObservers(); + test.equal(operations.shift(), ['removed', 1, 0, {a:3}]); + test.length(operations, 0); + h.stop(); }); diff --git a/packages/minimongo/observe.js b/packages/minimongo/observe.js index e12eee8ad4..38bcb4514c 100644 --- a/packages/minimongo/observe.js +++ b/packages/minimongo/observe.js @@ -193,11 +193,11 @@ LocalCollection._observeFromObserveChanges = function (cursor, observeCallbacks) // // NOTE: If called from an observe callback for a certain change, the result // is *not* guaranteed to be a snapshot of the cursor up to that - // change. This is because callbacks are deferred. + // change. This is because the callbacks are invoked before updating docs. handle._fetch = function () { var docsArray = []; changeObserver.docs.forEach(function (doc) { - docsArray.push(transform(doc)); + docsArray.push(transform(EJSON.clone(doc))); }); return docsArray; }; diff --git a/packages/minimongo/package.js b/packages/minimongo/package.js index f55acbf2b9..8791cbfe77 100644 --- a/packages/minimongo/package.js +++ b/packages/minimongo/package.js @@ -38,5 +38,6 @@ Package.on_test(function (api) { api.use(['tinytest', 'underscore', 'ejson', 'ordered-dict', 'random', 'deps']); api.add_files('minimongo_tests.js', 'client'); + api.add_files('wrap_transform_tests.js'); api.add_files('minimongo_server_tests.js', 'server'); }); diff --git a/packages/minimongo/sort.js b/packages/minimongo/sort.js index 718ec1537e..02218593ec 100644 --- a/packages/minimongo/sort.js +++ b/packages/minimongo/sort.js @@ -48,11 +48,19 @@ Sorter = function (spec) { // min/max.) // // XXX This is actually wrong! In fact, the whole attempt to compile sort - // functions independently of selectors is wrong. In MongoDB, if you have - // documents {_id: 'x', a: [1, 10]} and {_id: 'y', a: [5, 15]}, - // then C.find({}, {sort: {a: 1}}) puts x before y (1 comes before 5). - // But C.find({a: {$gt: 3}}, {sort: {a: 1}}) puts y before x (1 does not match - // the selector, and 5 comes before 10). + // functions independently of selectors is wrong. In MongoDB, if you have + // documents {_id: 'x', a: [1, 10]} and {_id: 'y', a: [5, 15]}, then + // C.find({}, {sort: {a: 1}}) puts x before y (1 comes before 5). But + // C.find({a: {$gt: 3}}, {sort: {a: 1}}) puts y before x (1 does not match + // the selector, and 5 comes before 10). + // + // The way this works is pretty subtle! For example, if the documents are + // instead {_id: 'x', a: [{x: 1}, {x: 10}]}) and + // {_id: 'y', a: [{x: 5}, {x: 15}]}), + // then C.find({'a.x': {$gt: 3}}, {sort: {'a.x': 1}}) and + // C.find({a: {$elemMatch: {x: {$gt: 3}}}}, {sort: {'a.x': 1}}) + // both follow this rule (y before x). ie, you do have to apply this + // through $elemMatch. var reduceValue = function (branchValues, findMin) { // Expand any leaf arrays that we find, and ignore those arrays themselves. branchValues = expandArraysInBranches(branchValues, true); diff --git a/packages/minimongo/wrap_transform.js b/packages/minimongo/wrap_transform.js index 92b46a15c8..f389c9d544 100644 --- a/packages/minimongo/wrap_transform.js +++ b/packages/minimongo/wrap_transform.js @@ -7,25 +7,29 @@ // - If the return value has an _id field, verify that it matches the // original _id field // - If the return value doesn't have an _id field, add it back. -LocalCollection.wrapTransform = function(transform) { +LocalCollection.wrapTransform = function (transform) { if (!transform) - return undefined; + return null; return function (doc) { - var id = doc._id; - var transformed = transform(doc); + if (!_.has(doc, '_id')) { + // XXX do we ever have a transform on the oplog's collection? because that + // collection has no _id. + throw new Error("can only transform documents with _id"); + } - if (typeof transformed !== 'object' || - transformed instanceof Array || - // Even though fine technically, don't let Mongo ObjectIDs - // through. It would suck to think your app works until - // you insert the first document using Meteor. - transformed instanceof Meteor.Collection.ObjectID) { + var id = doc._id; + // XXX consider making deps a weak dependency and checking Package.deps here + var transformed = Deps.nonreactive(function () { + return transform(doc); + }); + + if (!isPlainObject(transformed)) { throw new Error("transform must return object"); } - if (transformed._id) { - if (transformed._id !== id) { + if (_.has(transformed, '_id')) { + if (!EJSON.equals(transformed._id, id)) { throw new Error("transformed document can't have different _id"); } } else { diff --git a/packages/minimongo/wrap_transform_tests.js b/packages/minimongo/wrap_transform_tests.js new file mode 100644 index 0000000000..9cd73d2e21 --- /dev/null +++ b/packages/minimongo/wrap_transform_tests.js @@ -0,0 +1,58 @@ +Tinytest.add("minimongo - wrapTransform", function (test) { + var wrap = LocalCollection.wrapTransform; + + // Transforming no function gives falsey. + test.isFalse(wrap(undefined)); + test.isFalse(wrap(null)); + + // It's OK if you don't change the ID. + var validTransform = function (doc) { + delete doc.x; + doc.y = 42; + doc.z = function () { return 43; }; + return doc; + }; + var transformed = wrap(validTransform)({_id: "asdf", x: 54}); + test.equal(_.keys(transformed), ['_id', 'y', 'z']); + test.equal(transformed.y, 42); + test.equal(transformed.z(), 43); + + // Ensure that ObjectIDs work (even if the _ids in question are not ===-equal) + var oid1 = new LocalCollection._ObjectID(); + var oid2 = new LocalCollection._ObjectID(oid1.toHexString()); + test.equal(wrap(function () {return {_id: oid2};})({_id: oid1}), + {_id: oid2}); + + // transform functions must return objects + var invalidObjects = [ + "asdf", new LocalCollection._ObjectID(), false, null, true, + 27, [123], /adsf/, new Date, function () {}, undefined + ]; + _.each(invalidObjects, function (invalidObject) { + var wrapped = wrap(function () { return invalidObject; }); + test.throws(function () { + wrapped({_id: "asdf"}); + }); + }, /transform must return object/); + + // transform functions may not change _ids + var wrapped = wrap(function (doc) { doc._id = 'x'; return doc; }); + test.throws(function () { + wrapped({_id: 'y'}); + }, /can't have different _id/); + + // transform functions may remove _ids + test.equal({_id: 'a', x: 2}, + wrap(function (d) {delete d._id; return d;})({_id: 'a', x: 2})); + + // test that wrapped transform functions are nonreactive + var unwrapped = function (doc) { + test.isFalse(Deps.active); + return doc; + }; + var handle = Deps.autorun(function () { + test.isTrue(Deps.active); + wrap(unwrapped)({_id: "xxx"}); + }); + handle.stop(); +}); diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index 3f7173ce3e..9dff5cb778 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -38,11 +38,7 @@ Meteor.Collection = function (name, options) { break; } - if (options.transform) { - self._transform = Deps._makeNonreactive(Package.minimongo.LocalCollection.wrapTransform(options.transform)); - } else { - self._transform = null; - } + self._transform = LocalCollection.wrapTransform(options.transform); if (!name && (name !== null)) { Meteor._debug("Warning: creating anonymous collection. It will not be " + @@ -555,12 +551,16 @@ Meteor.Collection.ObjectID = LocalCollection._ObjectID; if (!(options[name] instanceof Function)) { throw new Error(allowOrDeny + ": Value for `" + name + "` must be a function"); } - if (self._transform && options.transform !== null) - options[name].transform = self._transform; - if (options.transform) - options[name].transform = Deps._makeNonreactive(options.transform); - if (options[name].transform) - options[name].transform = Package.minimongo.LocalCollection.wrapTransform(options[name].transform); + + // If the transform is specified at all (including as 'null') in this + // call, then take that; otherwise, take the transform from the + // collection. + if (options.transform === undefined) { + options[name].transform = self._transform; // already wrapped + } else { + options[name].transform = LocalCollection.wrapTransform( + options.transform); + } self._validators[name][allowOrDeny].push(options[name]); } diff --git a/packages/mongo-livedata/local_collection_driver.js b/packages/mongo-livedata/local_collection_driver.js index ec78544154..aa325979e5 100644 --- a/packages/mongo-livedata/local_collection_driver.js +++ b/packages/mongo-livedata/local_collection_driver.js @@ -5,7 +5,7 @@ LocalCollectionDriver = function () { var ensureCollection = function (name, collections) { if (!(name in collections)) - collections[name] = new LocalCollection(name); + collections[name] = new LocalCollection({name: name}); return collections[name]; }; diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js index a1140a6360..b3469a09be 100644 --- a/packages/mongo-livedata/mongo_driver.js +++ b/packages/mongo-livedata/mongo_driver.js @@ -778,9 +778,8 @@ var SynchronousCursor = function (dbCursor, cursorDescription, options) { // inside a user-visible Cursor, we want to provide the outer cursor! self._selfForIteration = options.selfForIteration || self; if (options.useTransform && cursorDescription.options.transform) { - self._transform = Deps._makeNonreactive( - Package.minimongo.LocalCollection.wrapTransform( - cursorDescription.options.transform)); + self._transform = LocalCollection.wrapTransform( + cursorDescription.options.transform); } else { self._transform = null; } diff --git a/packages/mongo-livedata/mongo_livedata_tests.js b/packages/mongo-livedata/mongo_livedata_tests.js index 86dd680b65..032e0dbaa7 100644 --- a/packages/mongo-livedata/mongo_livedata_tests.js +++ b/packages/mongo-livedata/mongo_livedata_tests.js @@ -818,42 +818,6 @@ testAsyncMulti('mongo-livedata - document with a date, ' + idGeneration, [ } ]); -testAsyncMulti('mongo-livedata - transform must return an object, ' + idGeneration, [ - function (test, expect) { - var self = this; - var justId = function (doc) { - return doc._id; - }; - var idInArray = function (doc) { - return [doc._id]; - }; - TRANSFORMS["justId"] = justId; - var collectionOptions = { - idGeneration: idGeneration, - transform: justId, - transformName: "justId" - }; - var collectionName = Random.id(); - if (Meteor.isClient) { - Meteor.call('createInsecureCollection', collectionName, collectionOptions); - Meteor.subscribe('c-' + collectionName); - } - self.coll = new Meteor.Collection(collectionName, collectionOptions); - self.coll.insert({}, expect(function (err, id) { - test.isFalse(err); - test.isTrue(id); - test.throws(function () { - self.coll.findOne(); - }); - test.throws(function () { - self.coll.findOne({}, {transform: idInArray}); - }); - // you can still override the transform though. - test.equal(self.coll.findOne({}, {transform: null}), {_id: id}); - })); - } -]); - testAsyncMulti('mongo-livedata - document goes through a transform, ' + idGeneration, [ function (test, expect) { var self = this; @@ -915,37 +879,6 @@ testAsyncMulti('mongo-livedata - document goes through a transform, ' + idGenera } ]); -testAsyncMulti('mongo-livedata - transformed object can\'t have conflicting _id, ' + idGeneration, [ - function (test, expect) { - var self = this; - var justId = function (doc) { - doc._id = "foo"; - return doc; - }; - TRANSFORMS["justId"] = justId; - var collectionOptions = { - idGeneration: idGeneration, - transform: justId, - transformName: "justId" - }; - var collectionName = Random.id(); - if (Meteor.isClient) { - Meteor.call('createInsecureCollection', collectionName, collectionOptions); - Meteor.subscribe('c-' + collectionName); - } - self.coll = new Meteor.Collection(collectionName, collectionOptions); - self.coll.insert({}, expect(function (err, id) { - test.isFalse(err); - test.isTrue(id); - test.throws(function () { - self.coll.findOne(); - }); - // you can still override the transform though. - test.equal(self.coll.findOne({}, {transform: null})._id, id); - })); - } -]); - testAsyncMulti('mongo-livedata - transform sets _id if not present, ' + idGeneration, [ function (test, expect) { var self = this; diff --git a/packages/mongo-livedata/oplog_observe_driver.js b/packages/mongo-livedata/oplog_observe_driver.js index 363ac59277..face62ccbb 100644 --- a/packages/mongo-livedata/oplog_observe_driver.js +++ b/packages/mongo-livedata/oplog_observe_driver.js @@ -30,13 +30,24 @@ OplogObserveDriver = function (options) { self._registerPhaseChange(PHASE.QUERYING); - self._published = new LocalCollection._IdMap; + // A minimongo LocalCollection containing the docs that match the selector, + // and maybe more. It is guaranteed to contain all the fields needed for the + // selector and the projection, and may have other fields too. (In the future + // we may try to make this collection be shared between multiple + // OplogObserveDrivers, but not currently.) + self._collection = + new LocalCollection({_observeCallbacksWillNeverYield: true}); + // XXX think about what all the options are + var minimongoCursor = self._collection.find( + self._cursorDescription.selector, self._cursorDescription.options); + self._stopHandles.push(minimongoCursor.observeChanges(self._multiplexer)); + var selector = self._cursorDescription.selector; self._matcher = options.matcher; - var projection = self._cursorDescription.options.fields || {}; - self._projectionFn = LocalCollection._compileProjection(projection); + // Projection function, result of combining important fields for selector and // existing fields projection + var projection = self._cursorDescription.options.fields || {}; self._sharedProjection = self._matcher.combineIntoProjection(projection); self._sharedProjectionFn = LocalCollection._compileProjection( self._sharedProjection); @@ -109,47 +120,51 @@ OplogObserveDriver = function (options) { _.extend(OplogObserveDriver.prototype, { _add: function (doc) { var self = this; - var id = doc._id; - var fields = _.clone(doc); - delete fields._id; - if (self._published.has(id)) - throw Error("tried to add something already published " + id); - self._published.set(id, self._sharedProjectionFn(fields)); - self._multiplexer.added(id, self._projectionFn(fields)); + doc = self._sharedProjectionFn(doc); + // XXX does _sharedProjection always preserve id? + if (!_.has(doc, '_id')) + throw Error("Can't add doc without _id"); + self._collection.insert(doc); }, - _remove: function (id) { + _remove: function (id, options) { var self = this; - if (!self._published.has(id)) + options = options || {}; + var removed = self._collection.remove({_id: id}); + if (options.mustExist && removed !== 1) throw Error("tried to remove something unpublished " + id); - self._published.remove(id); - self._multiplexer.removed(id); }, _handleDoc: function (id, newDoc, mustMatchNow) { var self = this; - newDoc = _.clone(newDoc); + newDoc = _.clone(newDoc); // *shallow* clone + // XXX this is just about "matching selector", not about skip/limit var matchesNow = newDoc && self._matcher.documentMatches(newDoc).result; if (mustMatchNow && !matchesNow) { throw Error("expected " + EJSON.stringify(newDoc) + " to match " + EJSON.stringify(self._cursorDescription)); } - var matchedBefore = self._published.has(id); + var inCollection = !!self._collection.find(id).count(); - if (matchesNow && !matchedBefore) { + if (matchesNow && !inCollection) { + // It matches the selector and it isn't in our collection, so add it. + // XXX once we add skip/limit, this may not always send an added, and + // we may need to do some GC self._add(newDoc); - } else if (matchedBefore && !matchesNow) { - self._remove(id); + } else if (inCollection && !matchesNow) { + // We remove this from the collection to achieve two goals: (a) causing + // the observeChanges to fire removed() and (b) saving memory. That said, + // it would be legitimate (if !!newDoc) to update the collection instead + // of removing, if we thought we might need this doc again soon. + self._remove(id, {mustExist: true}); } else if (matchesNow) { - var oldDoc = self._published.get(id); - if (!oldDoc) - throw Error("thought that " + id + " was there!"); - delete newDoc._id; - self._published.set(id, self._sharedProjectionFn(newDoc)); - var changed = LocalCollection._makeChangedFields(_.clone(newDoc), oldDoc); - changed = self._projectionFn(changed); - if (!_.isEmpty(changed)) - self._multiplexer.changed(id, changed); + // Replace the doc inside our collection, which may trigger a changed + // callback. + newDoc = self._sharedProjectionFn(newDoc); + // XXX does _sharedProjection always preserve id? + if (!_.has(newDoc, '_id')) + throw Error("Can't add newDoc without _id"); + self._collection.update(id, newDoc); } }, _fetchModifiedDocuments: function () { @@ -226,16 +241,16 @@ _.extend(OplogObserveDriver.prototype, { // If we're already fetching this one, or about to, we can't optimize; make // sure that we fetch it again if necessary. if (self._phase === PHASE.FETCHING && - (self._currentlyFetching.has(id) || self._needToFetch.has(id))) { + ((self._currentlyFetching && self._currentlyFetching.has(id)) || + self._needToFetch.has(id))) { self._needToFetch.set(id, op.ts.toString()); return; } if (op.op === 'd') { - if (self._published.has(id)) - self._remove(id); + self._remove(id); } else if (op.op === 'i') { - if (self._published.has(id)) + if (self._collection.find(id).count()) throw new Error("insert found for already-existing ID"); // XXX what if selector yields? for now it can't but later it could have @@ -257,18 +272,24 @@ _.extend(OplogObserveDriver.prototype, { if (isReplace) { self._handleDoc(id, _.extend({_id: id}, op.o)); - } else if (self._published.has(id) && canDirectlyModifyDoc) { - // Oh great, we actually know what the document is, so we can apply - // this directly. - var newDoc = EJSON.clone(self._published.get(id)); - newDoc._id = id; - LocalCollection._modify(newDoc, op.o); - self._handleDoc(id, self._sharedProjectionFn(newDoc)); - } else if (!canDirectlyModifyDoc || - self._matcher.canBecomeTrueByModifier(op.o)) { - self._needToFetch.set(id, op.ts.toString()); - if (self._phase === PHASE.STEADY) - self._fetchModifiedDocuments(); + } else { + var newDoc = self._collection.findOne(id); + if (newDoc && canDirectlyModifyDoc) { + // Oh great, we actually know what the document is, so we can apply + // this directly. + // XXX just send the modifier to _collection.update? but then + // we don't necessarily get to GC + + // We can avoid another deep clone here since the findOne above would + // return a copy anyways + LocalCollection._modify(newDoc, op.o); + self._handleDoc(id, newDoc); + } else if (!canDirectlyModifyDoc || + self._matcher.canBecomeTrueByModifier(op.o)) { + self._needToFetch.set(id, op.ts.toString()); + if (self._phase === PHASE.STEADY) + self._fetchModifiedDocuments(); + } } } else { throw Error("XXX SURPRISING OPERATION: " + op); @@ -317,18 +338,19 @@ _.extend(OplogObserveDriver.prototype, { self._currentlyFetching = null; ++self._fetchGeneration; // ignore any in-flight fetches self._registerPhaseChange(PHASE.QUERYING); + self._collection.pauseObservers(); + // XXX this won't be quite correct for skip/limit + self._collection.remove({}); // Defer so that we don't block. Meteor.defer(function () { - // subtle note: _published does not contain _id fields, but newResults - // does - var newResults = new LocalCollection._IdMap; - var cursor = self._cursorForQuery(); - cursor.forEach(function (doc) { - newResults.set(doc._id, doc); + // Insert all the documents currently found by the query. + self._cursorForQuery().forEach(function (doc) { + self._collection.insert(doc); }); - self._publishNewResults(newResults); + // Allow observe callbacks (ie multiplexer invocations) to fire. + self._collection.resumeObservers(); self._doneQuerying(); }); @@ -398,34 +420,6 @@ _.extend(OplogObserveDriver.prototype, { }, - // Replace self._published with newResults (both are IdMaps), invoking observe - // callbacks on the multiplexer. - // - // XXX This is very similar to LocalCollection._diffQueryUnorderedChanges. We - // should really: (a) Unify IdMap and OrderedDict into Unordered/OrderedDict (b) - // Rewrite diff.js to use these classes instead of arrays and objects. - _publishNewResults: function (newResults) { - var self = this; - - // First remove anything that's gone. Be careful not to modify - // self._published while iterating over it. - var idsToRemove = []; - self._published.forEach(function (doc, id) { - if (!newResults.has(id)) - idsToRemove.push(id); - }); - _.each(idsToRemove, function (id) { - self._remove(id); - }); - - // Now do adds and changes. - newResults.forEach(function (doc, id) { - // "true" here means to throw if we think this doc doesn't match the - // selector. - self._handleDoc(id, doc, true); - }); - }, - // This stop function is invoked from the onStop of the ObserveMultiplexer, so // it shouldn't actually be possible to call it until the multiplexer is // ready. @@ -449,7 +443,6 @@ _.extend(OplogObserveDriver.prototype, { self._writesToCommitWhenWeReachSteady = null; // Proactively drop references to potentially big things. - self._published = null; self._needToFetch = null; self._currentlyFetching = null; self._oplogEntryHandle = null; @@ -463,6 +456,9 @@ _.extend(OplogObserveDriver.prototype, { var self = this; var now = new Date; + if (phase === self._phase) + return; + if (self._phase) { var timeDiff = now - self._phaseStartTime; Package.facts && Package.facts.Facts.incrementServerFact( diff --git a/packages/spacebars-compiler/compile_tests.js b/packages/spacebars-compiler/compile_tests.js index c09afd02ba..d8858a1f99 100644 --- a/packages/spacebars-compiler/compile_tests.js +++ b/packages/spacebars-compiler/compile_tests.js @@ -106,7 +106,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), null, { + self.lookup("foo", {template: true}), null, { content: UI.block(function() { var self = this; return "abc"; @@ -132,7 +132,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return Spacebars.call(self.lookup("bar")); }); @@ -142,7 +142,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return { x: Spacebars.call(self.lookup("bar")) }; }); @@ -152,7 +152,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")); }); @@ -162,7 +162,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return { x: Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")) @@ -174,7 +174,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return Spacebars.dataMustache(self.lookup("bar"), self.lookup("baz")); @@ -185,7 +185,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return Spacebars.dataMustache(self.lookup("bar"), self.lookup("baz")); @@ -202,7 +202,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return Spacebars.include( - Template.foo || self.lookup("foo"), + self.lookup("foo", {template: true}), function () { return Spacebars.dataMustache( Spacebars.dot(self.lookup("p"), "q"), @@ -296,4 +296,4 @@ Tinytest.add("spacebars - compiler errors", function (test) { "First argument must be a function"); isError("{{#foo 0 x=0}}{{/foo}}", "First argument must be a function"); -}); \ No newline at end of file +}); diff --git a/packages/spacebars-compiler/spacebars-compiler.js b/packages/spacebars-compiler/spacebars-compiler.js index 204fc754fa..ffc78660bb 100644 --- a/packages/spacebars-compiler/spacebars-compiler.js +++ b/packages/spacebars-compiler/spacebars-compiler.js @@ -203,17 +203,9 @@ var codeGenTemplateTag = function (tag) { builtInBlockHelpers[path[0]] + '(' + callArgs.join(', ') + ')'); } else { - var compCode = codeGenPath(path); + var compCode = codeGenPath(path, {lookupTemplate: true}); - if (path.length === 1) { - // toObjectLiteralKey returns `"foo"` or `foo` depending on - // whether `foo` is a safe JavaScript identifier. - var member = toObjectLiteralKey(path[0]); - var templateDotFoo = (member.charAt(0) === '"' ? - 'Template[' + member + ']' : - 'Template.' + member); - compCode = ('(' + templateDotFoo + ' || ' + compCode + ')'); - } else { + if (path.length !== 1) { // path code may be reactive; wrap it compCode = 'function () { return ' + compCode + '; }'; } @@ -252,7 +244,14 @@ var makeObjectLiteral = function (obj) { // (i.e. it may invalidate the current computation). // // No code is generated to call the result if it's a function. -var codeGenPath = function (path) { +// +// Options: +// +// - lookupTemplate {Boolean} If true, generated code also looks in +// the list of templates. (After helpers, before data context). +// Used when generating code for `{{> foo}}` or `{{#foo}}`. Only +// used for non-dotted paths. +var codeGenPath = function (path, opts) { if (builtInBlockHelpers.hasOwnProperty(path[0])) throw new Error("Can't use the built-in '" + path[0] + "' here"); // Let `{{#if content}}` check whether this template was invoked via @@ -264,7 +263,10 @@ var codeGenPath = function (path) { return builtInLexicals[path[0]]; } - var code = 'self.lookup(' + toJSLiteral(path[0]) + ')'; + var args = [toJSLiteral(path[0])]; + if (opts && opts.lookupTemplate && path.length === 1) + args.push('{template: true}'); + var code = 'self.lookup(' + args.join(', ') + ')'; if (path.length > 1) { code = 'Spacebars.dot(' + code + ', ' + diff --git a/packages/spacebars-tests/template_tests.html b/packages/spacebars-tests/template_tests.html index a5c4da1552..602c647b21 100644 --- a/packages/spacebars-tests/template_tests.html +++ b/packages/spacebars-tests/template_tests.html @@ -448,3 +448,20 @@ Hi there! + + + + + + + + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index d74ec312a2..4b1e29cd67 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1340,3 +1340,28 @@ Tinytest.add("spacebars - templates - double", function (test) { run(null, ''); run(undefined, ''); }); + +Tinytest.add("spacebars - templates - inclusion lookup order", function (test) { + // test that {{> foo}} looks for a helper named 'foo', then a + // template named 'foo', then a 'foo' field in the data context. + var tmpl = Template.spacebars_template_test_inclusion_lookup; + tmpl.data = function () { + return { + // shouldn't have an effect since we define a helper with the + // same name. + spacebars_template_test_inclusion_lookup_subtmpl: Template. + spacebars_template_test_inclusion_lookup_subtmpl3, + dataContextSubtmpl: Template. + spacebars_template_test_inclusion_lookup_subtmpl3}; + }; + + tmpl.spacebars_template_test_inclusion_lookup_subtmpl = + Template.spacebars_template_test_inclusion_lookup_subtmpl2; + + var lines = _.map( + stripComments(renderToDiv(tmpl).innerHTML).split('\n'), + trim); + test.equal(lines, + ["This is generated by a helper with the same name.", + "This is a template passed in the data context."]); +}); diff --git a/packages/ui/fields.js b/packages/ui/fields.js index 3bcf1b81d3..4146c5fe8a 100644 --- a/packages/ui/fields.js +++ b/packages/ui/fields.js @@ -29,8 +29,13 @@ var builtInComponents = { }; _extend(UI.Component, { - lookup: function (id) { + // Options: + // + // - template {Boolean} If true, look at the list of templates after + // helpers and before data context. + lookup: function (id, opts) { var self = this; + var template = opts && opts.template; var result; var comp; @@ -58,6 +63,7 @@ _extend(UI.Component, { } else if (_.has(builtInComponents, id)) { return builtInComponents[id]; + // Code to search the global namespace for capitalized names // like component classes, `Template`, `StringUtils.foo`, // etc. @@ -79,6 +85,10 @@ _extend(UI.Component, { // for this? We should definitely not put it on the Handlebars // namespace. result = Handlebars._globalHelpers[id]; + + } else if (template && _.has(Template, id)) { + return Template[id]; + } else { // Resolve id `foo` as `data.foo` (with a "soft dot"). return function (/*arguments*/) { diff --git a/packages/underscore-tests/.gitignore b/packages/underscore-tests/.gitignore new file mode 100644 index 0000000000..677a6fc263 --- /dev/null +++ b/packages/underscore-tests/.gitignore @@ -0,0 +1 @@ +.build* diff --git a/packages/underscore-tests/each_test.js b/packages/underscore-tests/each_test.js new file mode 100644 index 0000000000..50b071ee8d --- /dev/null +++ b/packages/underscore-tests/each_test.js @@ -0,0 +1,45 @@ +Tinytest.add("underscore - each", function (test) { + // arrays + _.each([42], function (val, index) { + test.equal(index, 0); + test.equal(val, 42); + }); + + // objects with 'length' field aren't treated as arrays + _.each({length: 42}, function (val, key) { + test.equal(key, 'length'); + test.equal(val, 42); + }); + + // The special 'arguments' variable is treated as an + // array + (function () { + _.each(arguments, function (val, index) { + test.equal(index, 0); + test.equal(val, 42); + }); + })(42); + + // An object with a 'callee' field isn't treated as arguments + _.each({callee: 42}, function (val, key) { + test.equal(key, 'callee'); + test.equal(val, 42); + }); + + // An object with a 'callee' field isn't treated as arguments + _.each({length: 4, callee: 42}, function (val, key) { + if (key === 'callee') + test.equal(val, 42); + else if (key === 'length') + test.equal(val, 4); + else + test.fail({message: 'unexpected key: ' + key}); + }); + + + // NOTE: An object with a numberic 'length' field *and* a function + // 'callee' field will be treated as an array in IE. This may or may + // not be fixable, but isn't a big deal since: (1) 'callee' is a + // pretty rare key, and (2) JSON objects can't have functions + // anyways, which is the main use-case for _.each. +}); diff --git a/packages/underscore-tests/package.js b/packages/underscore-tests/package.js new file mode 100644 index 0000000000..0eb7218126 --- /dev/null +++ b/packages/underscore-tests/package.js @@ -0,0 +1,10 @@ +Package.describe({ + // These tests can't be directly in the underscore packages since + // Tinytest depends on underscore + summary: "Tests for the underscore package" +}); + +Package.on_test(function (api) { + api.use(['tinytest', 'underscore']); + api.add_files('each_test.js'); +}); diff --git a/packages/underscore/underscore.js b/packages/underscore/underscore.js index 12a74d5f55..f5ca16aa33 100644 --- a/packages/underscore/underscore.js +++ b/packages/underscore/underscore.js @@ -70,6 +70,21 @@ // Collection Functions // -------------------- + // METEOR CHANGE: Define _isArguments instead of depending on + // _.isArguments which is defined using each. In looksLikeArray + // (which each depends on), we then use _isArguments instead of + // _.isArguments. + var _isArguments = function (obj) { + return toString.call(obj) === '[object Arguments]'; + }; + // Define a fallback version of the method in browsers (ahem, IE), where + // there isn't any inspectable "Arguments" type. + if (!_isArguments(arguments)) { + _isArguments = function (obj) { + return !!(obj && hasOwnProperty.call(obj, 'callee') && typeof obj.callee === 'function'); + }; + } + // METEOR CHANGE: _.each({length: 5}) should be treated like an object, not an // array. This looksLikeArray function is introduced by Meteor, and replaces // all instances of `obj.length === +obj.length`. @@ -77,7 +92,8 @@ // https://github.com/jashkenas/underscore/issues/770 var looksLikeArray = function (obj) { return (obj.length === +obj.length - && (_.isArguments(obj) || obj.constructor !== Object)); + // _.isArguments not yet necessarily defined here + && (_isArguments(obj) || obj.constructor !== Object)); }; // The cornerstone, an `each` implementation, aka `forEach`. diff --git a/scripts/generate-dev-bundle.sh b/scripts/generate-dev-bundle.sh index bbee7da6e6..8270888a5b 100755 --- a/scripts/generate-dev-bundle.sh +++ b/scripts/generate-dev-bundle.sh @@ -109,11 +109,7 @@ npm install eachline@2.4.0 npm install source-map@0.1.31 npm install source-map-support@0.2.5 npm install bcrypt@0.7.7 - -# Based on 1.0.1; includes our PRs -# https://github.com/nodejitsu/node-http-proxy/pull/561 and -# https://github.com/nodejitsu/node-http-proxy/pull/560 -npm install https://github.com/meteor/node-http-proxy/tarball/d8ea687936d6bed0f3e99849695cab2dcdccd6f4 +npm install http-proxy@1.0.2 # Using the unreleased 1.1 branch. We can probably switch to a built NPM version # when it gets released.