No longer using a RegExp when we know what the old file wildcard path
should be, and no longer using Fiber when we can just use a Promise
callback (since all Promise callbacks run in a Fiber).
This method appears to be causing large spikes in memory consumption on
Circle CI during the `meteor --get-ready` preparation step, which often
leads to the test process being killed.
Also added a call in IsopackCache#_loadLocalPackage for good measure.
We're now calling requestGarbageCollection pretty frequently when
we run Node with --expose-gc, but that currently only happens during
Circle CI tests, so I don't think we need to implement the improvements
suggested in tools/utils/gc.js, yet.
Previously: 35f488e140, f6df21ff1e
To deal with individual flaky tests, we often just re-run the entire test
suite, which feels like an enormous waste of shared computing resources.
This change automatically re-runs individual failed tests as many as two
more times, and considers the test successful if any of those attempts
succeeds.
cc @abernix @hwillson et al.
Now that dynamic modules are part of the manifest that determines which
files are served over HTTP, I'm a bit paranoid about them somehow ending
up as <script> tags in the initial HTML of the application.
This commit adds another safety measure to prevent that, just in case the
boilerplate-generator package for some reason fails to skip items whose
.path starts with "dynamic/" (see my previous commit).
This allows fetching the compiled code of dynamic modules via HTTP,
without generating <script> tags for those resources in the intial
boilerplate HTML of the application.
The URL for a dynamic module should be formed by taking its absolute
module identifier, prepending "/dynamic" and appending "?hash=<version>".
Appropriate version hashes can be obtained from the tree exported by the
meteor/dynamic-import/dynamic-versions.js module, though the hashes are
used only for cache busting, so they could be anything at all.
A good place to do this fetching would be the meteorInstall.fetch
callback, as defined (for example) in meteor/dynamic-import/client.js.
That implementation still uses a WebSocket rather than HTTP, but this
commit will allow us to experiment with HTTP in the future.
Because the code returned for these dynamic modules is wrapped as an
anonymous function expression, you'll need to fetch them using an
XMLHttpRequest, the HTTP fetch() function, or some similar utility, rather
than using a <script> tag, because executing the unmodified code as JS
will likely throw a syntax error.
Partially reverts commit 8364f81344.
This commit was causing self-test failures like the following:
While selecting package versions:
error: unknown package in top-level dependencies: dynamic-import
I suspect these tests won't work until the dynamic-import package is
published with a non-prerelease version.
It's tempting to make the ecmascript package api.imply("dynamic-import"),
but the dynamic-import package depends on the ddp package, which depends
heavily on the ecmascript package, and I'm not sure how best to break that
dependency cycle.
Now anyone can define meteorInstall.fetch however they see fit, and the
install.js implementation will handle everything else.
This separation of concerns leads to significantly less code, too.
As proposed here: https://github.com/rollup/rollup/wiki/pkg.module
By supporting ECMAScript module entry points for npm packages in Meteor
1.5, we will be well-positioned to do more effective import/export-based
tree shaking in future versions of Meteor.
We can't do the same thing on the server because we can't change how
native Node resolves package entry points based on the "main" field of the
package.json module.
On the other hand, all npm packages have to work in Node using the "main"
field, and client bundles stand to benefit the most from tree shaking, so
this client/server difference should not be problematic.
Note that the "jsnext:main" property is also supported as a legacy synonym
for "module".
* Implement CORDOVA_COMPATIBILITY_VERSION_EXCLUDE and CORDOVA_COMPATIBILITY_VERSION_IOS/ANDROID
CORDOVA_COMPATIBILITY_VERSION_IOS or CORDOVA_COMPATIBILITY_VERSION_ANDROID allows to override compatibility version for a specified platform.
CORDOVA_COMPATIBILITY_VERSION_EXCLUDE provides a way of excluding a certain plugin from compatibility version calculation. You can pass several plugin names with ';'. For example: `CORDOVA_COMPATIBILITY_VERSION_EXCLUDE='cordova-plugin-crosswalk-webview;cordova-plugin-device'`
* Changes after review
The comma in question: a trailing comma a rest-parameter, within a
function argument's parameter de-structuring:
function a({
a = false,
...b,
}) {
// ...
}
Espree, the parser used by `jsdoc` (used in Meteor docs) previously
allowed this with `experimentalObjectRestSpread` enabled but now throws
an error with the addition of 652990a7bf.
It might have been argued at one point that the trailing-comma could
allow for the easy, future addition of another parameter, ala:
function a({
a = false,
...b,
c = true,
}) {
// ...
}
Having the rest-parameter in the last position is certainly more clear
(it is the "rest", after-all, and there can be only one) but I'm not
going to have that discussion at the cost of docs not deploying!
Generally, module.prefetch(id) will not throw even if the fetched module
is missing. If you need to know whether module.prefetch(id) succeeded,
simply await the result of the promise, which will be null on success, or
an Error object if the module could not be imported.
Previously, the linker included a "version" hash in each stub entry
corresponding to a dynamic module in the meteorInstall bundle. These stub
entries also contain information about any module identifiers ("deps")
imported by the module, and various other metadata.
This hash was derived from the source code of the dynamic module, instead
of its fully-processed generated code, which created a small risk that the
source hash would remain the same when anything changed in the post-linker
processing logic.
This new implementation uses the same hashes found in program.json
manifest files, which more reliably reflect changes in the actual final
contents of the modules.
Because these hashes are not known at link time, they are now injected as
one blob into the meteor/dynamic-import/dynamic-versions.js module, rather
than appearing in the meteorInstall bundles for their containing packages.
This polyfill is unnecessary in Node, and added a whopping 22KB to the
minified client bundle. If you really need the Buffer API on the client,
you can get it from require("buffer").Buffer.
The comma in question: a trailing comma a rest-parameter, within a
function argument's parameter de-structuring:
function a({
a = false,
...b,
}) {
// ...
}
Espree, the parser used by `jsdoc` (used in Meteor docs) previously
allowed this with `experimentalObjectRestSpread` enabled but now throws
an error with the addition of 652990a7bf.
It might have been argued at one point that the trailing-comma could
allow for the easy, future addition of another parameter, ala:
function a({
a = false,
...b,
c = true,
}) {
// ...
}
Having the rest-parameter in the last position is certainly more clear
(it is the "rest", after-all, and there can be only one) but I'm not
going to have that discussion at the cost of docs not deploying!
This would have been a nice optimization if it had worked, but (in
addition to leaving garbage directories lying around sometimes if the
process was killed), it appears to have some unintended consequences for
meteorNpm.rebuildIfNonPortable and related functions, since the garbage
directories are easily confused for npm package directories.
Example stack trace:
Error: ENOENT: no such file or directory, open '/home/travis/build/meteor/galaxy-server/node_modules/heapdump-garbage-1c2jqib/package.json'
at Error (native)
=> awaited here:
at Promise.await (/home/travis/.meteor/packages/less/.2.7.9.9fh5c1++os+web.browser+web.cordova/plugin.compileLessBatch.os/npm/node_modules/meteor/promise/node_modules/meteor-promise/promise_server.js:39:12)
at copyFileHelper (/tools/fs/files.js:633:6)
at Object.files.cp_r (/tools/fs/files.js:532:7)
at /tools/isobuild/meteor-npm.js:393:11
at Array.forEach (native)
at copyNpmPackageWithSymlinkedNodeModules (/tools/isobuild/meteor-npm.js:386:29)
at /tools/isobuild/meteor-npm.js:325:5
at Array.forEach (native)
at Object.rebuildIfNonPortable (/tools/isobuild/meteor-npm.js:315:17)
at NodeModulesDirectory.rebuildIfNonPortable (/tools/isobuild/bundler.js:273:22)
at /tools/isobuild/compiler.js:650:13
This also makes it possible to disable it in places where we were not
checking against the `METEOR_DISABLE_FS_FIBERS` env. variable, like in
`rm_recursive`.
The `rename` is not sufficient on its own since it won't pave the way
for the new directory by ensuring that it's empty first.
`renameDirAlmostAtomically` will do that.
Unfortunately, `fs-extra` is still not as perfect at handling various
file system conditions as would be ideal. It seemed sensical to try and
use a library like this however, it turns out that the Meteor suite
of file system functions stands up best on Windows, which is where I
encountered most problems.
For example, `fs-extra` still tries to create symlinks as an unprivileged
user – a forbidden task on Windows unless running as Administrator.
In addition, I ran into a constant stream of other errors: `ENOTEMPTY`,
`EBUSY`, `EEXIST` – all for various reasons.
My current recommendation is that we remove `fs-extra` and replace the
`Builder#complete` `renameDirAlmostAtomically` call (which does not
absolutely _have_ to be done atomically) with a `try`/`catch` which
resorts to a basic copy if `err.code === 'EXDEV'`. All other
functionality stays the same.
This reverts commits:
* d49f3e2704
* 3257bafc84
* 74cb8ebdc2
* 5bbdcc9baa
* 6a0767bbac
Previously, we captured and displayed shorter error messages for the
more complicated and unnecessarily verbose messages which npm produced.
While updating npm to 4.4.4, I observed the changelog for 4.4.0
indicated it would now produce less verbose messages. In searching for
possible Meteor conflicts with this, I discovered that
`installNpmModule` had already regressed on providing pretty messages.
This fixes those messages to be parsed properly and adds tests which
ensure if npm changes again that we can capture them.
Follows-up on: https://github.com/meteor/meteor/pull/8562
* Improve `fs-extra.move` calls for Windows platform.
This is a follow-up to meteor/meteor#8491 which worked properly on Unix
platforms, but failed in a variety of ways on Windows due to its lack
of Fiber-awareness and desire to create symlinks as unprivileged users
(something not always possible on Windows).
The Fiber issue was observed when trying to remove "src" directories
within the `move` function (which tries a variety of OS/OS/arch-specific
techniques to accomplish its goal) after they had been copied to "dest".
On Windows, this resulted in `EDIRNOTEMPTY` errors since Windows appears
to temporarily cache the file-handle or doesn't release the file-handle
until the next tick.
The symlink issue will hopefully improve in an upcoming release of
Windows (Creator Edition) when Microsoft makes it possible to create
symlinks as an unprivileged user, however it will still require enabling
"Developer" mode in Windows settings. This implements the same catch
which was already in place for `fs.rename` on the `fs.move` provided by
`fs-extra`.
Performance gains were the same in tests comparing before and after
these changes.
Relates to:
https://github.com/meteor/meteor/issues/8558#issuecomment-291194385
* A few code-cleanups to my original commit.
Instead of fetching a bundle of JavaScript from the server, Cordova apps
include the complete bundle in the installed app, so there's no compelling
reason to allow dynamic module fetching.
The dynamic import(...) function will still work on Cordova, of course.
It will just never have to fetch anything from the server.
