always clone port list

[ktmud]

This is because config ports are sealed when required as a module. Under node v0.11.14, it will throw an error when you try to modify a sealed Array.

[es128]

You're passing in a custom ports array?

[es128]

Anyway, couldn't this be fixed just by changing cfg.port to cfg.port.slice() without the rest of the refactoring you did?

[ktmud]

sure it can.

[ktmud]

diff --git a/index.js b/index.js
index bfd0219..4df7017 100644
--- a/index.js
+++ b/index.js
@@ -4,9 +4,6 @@ var isWorker = require('cluster').isWorker;
 var isCss = function(file) {
   return sysPath.extname(file.path) === '.css';
 };
-var DEFAULT_PORTS = [
-  9485, 9486, 9487, 9488, 9489, 9490, 9491, 9492, 9493, 9494, 9495
-]

 function AutoReloader(config) {
   if (config == null) config = {};
@@ -16,14 +13,18 @@ function AutoReloader(config) {
   }
   var plugins = config.plugins || {};
   var cfg = plugins.autoReload || {};
+  var ports;
+
+  if (cfg.port == null) {
+    ports = [9485, 9486, 9487, 9488, 9489, 9490, 9491, 9492, 9493, 9494, 9495];
+  } else {
+    ports = Array.isArray(cfg.port) ? cfg.port.slice() : [cfg.port];
+  }
+
   if (this.config.persistent) {
     this.enabled = (cfg.enabled == null) ? true : cfg.enabled;
   }
   this.delay = cfg.delay;
-  var ports = cfg.port || DEFAULT_PORTS;
-  if (!Array.isArray(ports)) ports = [ports];
-
-  ports = ports.slice();

   var conns = this.connections = [];
   var port = this.port = ports.shift();

then it has to be a bigger refactor...

[es128]

ah, good point, we don't know yet at that point whether it is an array

[es128]

oh whoops you changed the PR... I was going to take the first solution after you pointed out the flaw in my thinking

No problem, I can tweak it on my own... Thanks for pointing out the bug

[ktmud]

:) I just thought the latter solution also look kinda nice. haha.

[es128]

This is because config ports are sealed when required as a module.

btw, just a correction on this point - brunch deliberately does an Object.freeze on the config object before passing it into any plugins. So I'm not sure if you misunderstood why this was happening, or I'm just misunderstanding your wording. But it's not dependent on a particular node version.

I'm just not sure that anyone setting a custom port has bothered to provide their own array before as opposed to setting a particular one (which would not have been impacted by the bug).

[es128]

I just thought the latter solution also look kinda nice

It does. Actually my biggest gripe with it at this point is the array of numbers that would be better handled by incrementing. It had looked nicer in coffeescript.

[ktmud]

I believe an older version node will not throw the error. Upgraded node today and saw the exception. Didn't dig in too much.

Maybe you shouldn't provide the Array api at all, if you believe the user would be good with only setting a specific port. I just copied the example configuration in the doc (lazy..), that's why I bump into this bug.

[es128]

Yeah looks like you're right, the Object.freeze behavior did change somewhere along the way in 0.11. In 0.11.2 (which I happened to have via nvm) it allows a frozen array to be shifted, while in 0.11.14 it does not. I'm not curious enough to keep digging to pinpoint exactly when the change began.

Regarding allowing the array as an option, I believe it is useful in case someone wants to run brunch on several projects simultaneously - having specified a known port range for the auto-reload websocket server. My assertions about why this bug hadn't been experience/reported before were based on incorrect assumptions.