Remove deprecated code from before Meteor 1.0 that was documented via XXX COMPAT in comments. Move deprecated packages into the deprecated folder (deps, livedata, srp).
Commit 646fa4e3eefixed#10547 by
restricting optimisticLookupPackageJson to package.json files with a
"name" property, which effectively skipped over intermediate package.json
files with additional properties.
However, in Node.js 12.16.0 (Meteor 1.9.1+), modules evaluated natively by
Node are considered ECMAScript modules if the closest package.json file
has "type": "module" (or has an .mjs file extension). This poses a problem
for the module.useNode() trick (see packages/modules-runtime/server.js),
because ESM modules cannot be imported using require.
For example, recent versions of the @babel/runtime package have a
@babel/runtime/helpers/esm/package.json file for the ESM versions of its
helpers (which specifies "type": "module"), but that package.json file
does not have a "name" property, because it is not the root package.json
file representing the entire @babel/runtime package.
I considered making the "name" restriction configurable, but that would
have fragmented the caching of optimisticLookupPackageJson. Instead, I
made it return an array of all potentially relevant package.json objects,
which can be safely cached.
This means that the caller has to iterate over the array, but there is
only one call site for this function (in tools/isobuild/package-source.js)
right now, so that wasn't too much work.
source-map 0.7.0+ has a much faster Rust WASM implementation.
Because this needs to be loaded, the constructor is now asynchronous.
The consumer should also be destroyed after it's no longer needed.
https://github.com/meteor/meteor/pull/10772#issuecomment-553517459
The assertion in tools/fs/optimistic.ts was failing if I passed a relative
path for --test-app-path, and passing the path as a second argument when
calling assert made it easier to tell what was going on, so I decided to
keep that change.
Now that files.rename uses Promise.prototype.await on Windows, it's
important to be sure it never gets called outside of a Fiber. Though we
don't run our full test suite on Windows, we can validate this expectation
by wrapping files.rename on all platforms. This commit should be reverted
once the validation is complete.
Falling back to a full recursive copy was MUCH more expensive than waiting
a short amount of time before retrying the rename.
This aligns with the way graceful-fs handles EPERM and EACCES errors on
Windows: https://www.npmjs.com/package/graceful-fs#improvements-over-fs-module
Using fs.writeFileSync in a serial style becomes especially costly when
we're writing a lot of files. In a recent profiling exercise I did on
Windows, nearly 80% of the time taken by Builder#_copyDirectory was spent
just closing the written files. By using the asynchronous fs.writeFile
function, we should be able to parallelize at least some of this work, and
await all the promises at the very end of copying the directory.
In PR #10720, we introduced the makeCheapPathFunction in an effort to
reduce the caching overhead for very frequently called (and already pretty
quick) operations like files.stat.
However, the default maximum LRU cache size of Math.pow(2, 16) can cause
quite a bit of cache eviction churn for large applications, which @veered
has identified as a potential source of build performance problems.
By setting the maximum cache size to Math.pow(2, 20) instead, I am no
longer seeing any files.stat calls in the profiling output for rebuilding
a large internal app, saving several seconds of rebuild time. The obvious
downside is that this cache might accumulate more memory over time, which
is why I didn't just set the max to Infinity, though that might be a
viable option if the total set of paths ever stat'd is small enough to fit
into the available memory.
In the future, I hope to find ways of managing LRU cache size that respond
to actual memory pressure (relative to available memory), rather than
pruning the cache after an arbitrary numeric threshold is reached.
Partial alternative to using json5, which we tried in commit
5124cb495c, which was recently reverted due
to performance concerns: 0e19c365a8.
Should solve the specific problem reported in #10688.
This reverts commit 5124cb495c.
Perhaps unsurprisingly, JSON5 is much slower than JSON, so much so that
"other optimisticReadJsonOrNull" has become the single most expensive leaf
when building one of our internal applications. We will have to find
another solution.
This function determines whether the optimistic caching system should
subscribe to file change notifications for a given path.
Note that Meteor has other ways of watching for file changes (e.g. the
WatchSet abstraction), as well as other mechanisms for invalidating cached
results, so time and resources can often be saved by returning false here,
if we know that the path will be watched in other ways.
Improvements:
- Add METEOR_PROFILE instrumentation to the shouldWatch implementation.
- Stop exporting shouldWatch, since it is no longer used anywhere else in
the codebase.
- Return false early to avoid watching files inside the .meteor directory,
a significant shortcut.
- Allow watching files in application node_modules directories that are
not located directly in the root application directory (closes#10664).
Because json5 is more tolerant of non-standards-compliant input, it has no
trouble with \t tab characters outside of string literals, so this change
should fix issue #10688.
Note that json5 is already installed in dev_bundle/lib/node_modules/json5,
because it is a dependency of @babel/core, so we don't need to rebuild the
dev bundle immediately, though it would be a good idea to do so before the
next beta/RC release.
The previous implementation simply avoided calling watchSet.addFile for
potentially unused files, trusting that addFile would be called later if
the file was eventually used. However, this strategy left the contents of
watchSet.files incomplete for tasks such as IsopackCache._checkUpToDate,
which require full information about all files, even the ones that might
not be used by the bundle. The new strategy maintains metadata about
potentially unused files in a separate data structure, which will be
merged/cloned/serialized/deserialized along with other WatchSet data.
Unfortunately, this conversion triggered an error due to one of the
shortcomings of the Babel implementation of TypeScript:
SyntaxError: /tools/tool-env/profile.ts: Namespace not marked type-only declare. Non-declarative namespaces are only supported experimentally in Babel. To enable and review caveats see: https://babeljs.io/docs/en/babel-plugin-transform-typescript
278 | }
279 |
> 280 | export namespace Profile {
| ^
281 | export let enabled = !! process.env.METEOR_PROFILE;
282 |
283 | export function time<TResult>(bucket: string, f: () => TResult) {
at File.buildCodeFrameError (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/core/lib/transformation/file/file.js:261:12)
at transpileNamespace (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/plugin-transform-typescript/lib/namespace.js:25:25)
at PluginPass.TSModuleDeclaration (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/plugin-transform-typescript/lib/index.js:271:32)
at newFn (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/visitors.js:193:21)
at NodePath._call (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/path/context.js:53:20)
at NodePath.call (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/path/context.js:40:17)
at NodePath.visit (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/path/context.js:88:12)
at TraversalContext.visitQueue (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/context.js:118:16)
at TraversalContext.visitMultiple (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/context.js:85:17)
at TraversalContext.visit (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/context.js:144:19)
