The `child_process` documentation indicates that .cmd scripts (such as
the `npm.cmd` script for npm) must be started within a shell, such as
cmd.exe.
While it was tempting to switch this to use `child_process`'s `spawn`,
which supports a `shell` option, it would have required us to buffer
our own stdout/stderr output via `data` event handlers.
Ref: https://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows
This was causing problems with `npm@5`, but realistically it may not be
be necessary anymore since `npm@5` has a much smarter global,
self-healing `cacache`.
@GeoffreyBooth and I are still deciding exactly what to do with the
coffeescript package in #8960, but in the meantime I need to publish
another 1.6 beta, and I'd like to avoid publishing a beta version of
coffeescript along with it.
Declaring a package-local variable called `process` in the `modules`
package causes uglify-js not to replace `process.env.NODE_ENV` with a
string literal value in any `node_modules` contained by the `modules`
package, which causes React Dev Tools to display a warning.
This Meteor package version does not need to track the npm version of the
coffeescript package, and probably should not change as often as the
version of the packages/non-core/coffeescript-compiler package.
They depend on core packages like caching-compiler, but coffeescript-compiler
can remain in non-core, so it can update more frequently as npm coffeescript
gets updated.
This fixes a problem reported by @arggh:
https://github.com/meteor/meteor/pull/8913#issuecomment-322048927
The problem appears to have been introduced by this commit:
2c5094fa0c
The reason this commit was problematic is that it turned several
Object.keys/forEach loops into for-in loops, which caused a few unchanged
return statements to take on a new meaning: instead of "continuing" the
loop by returning from the forEach callback function, the return
statements now returned from the enclosing function:
2c5094fa0c (commitcomment-23638205)2c5094fa0c (commitcomment-23638216)
Besides this refactoring hazard, what else is wrong with for-in loops?
First, for-in loops iterate over not only own but also inherited keys,
which makes them not exactly equivalent to Object.keys/forEach, which
iterates over only own keys. Another way to address that problem is to
initialize this.queries to Object.create(null) instead of {} (which I've
also done in this commit), so that there is no prototype chain.
Second, for-in loops are a trigger for deoptimization in V8:
https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#5-for-in
Since 2c5094fa0c was supposed to be about
improving performance, and these changes introduced some logical problems
as well, I think it's best that we avoid for-in loops.
Should fix#8988 and #8942.
Now that #8866 is the default behavior, it can take up to 5000ms for
changes to files modified during the build process to be noticed.
Before #8866, when we called e.g. files.writeFile(path), a native file
watcher would notice the change immediately, almost always before the
build process read the file again. This was definitely racy, but we were
getting away with it consistently... until #8866.
I was able to reproduce the problem in #8988 by running
echo some-local-package-name >> .meteor/packages
in an app with a local package of the given name. After debugging the
endless rebuild cycle, I found that .meteor/versions was being rewritten
by files.writeFile during the build process, but the file watching system
was not noticing the change in time to prevent watch.isUpToDate from
returning true. The change was finally detected when restarting the
Watcher responsible for .meteor/versions, which of course triggered
another rebuild, so the same problem kept happening again and again.
In retrospect, I could have tested this new issue template formatting by copying and pasting into the issue textarea, but I didn't. Sorry for the incremental commits on `devel`. This should be the last one. :)
