Skip to content

Commit

Permalink
fix(zone.js): patch fs.realpath.native as macrotask (#54208)
Browse files Browse the repository at this point in the history
This commit updates the implementation of the zone.js `fs` patch to
restore the implementation of `realpath.native` and patches it as a macrotask,
along with other functions of the `fs` package. This is the only nested function
that must be patched.

Closes: #45546

PR Close #54208
  • Loading branch information
arturovt authored and thePunderWoman committed Feb 6, 2024
1 parent ca45917 commit 4297005
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 14 deletions.
39 changes: 26 additions & 13 deletions packages/zone.js/lib/node/fs.ts
Expand Up @@ -6,15 +6,17 @@
* found in the LICENSE file at https://angular.io/license
*/

import {patchMacroTask} from '../common/utils';
import {patchMacroTask, zoneSymbol} from '../common/utils';

Zone.__load_patch('fs', () => {
Zone.__load_patch('fs', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
let fs: any;
try {
fs = require('fs');
} catch (err) {
}

if (!fs) return;

// watch, watchFile, unwatchFile has been patched
// because EventEmitter has been patched
const TO_PATCH_MACROTASK_METHODS = [
Expand All @@ -25,17 +27,28 @@ Zone.__load_patch('fs', () => {
'symlink', 'truncate', 'unlink', 'utimes', 'write', 'writeFile',
];

if (fs) {
TO_PATCH_MACROTASK_METHODS.filter(name => !!fs[name] && typeof fs[name] === 'function')
.forEach(name => {
patchMacroTask(fs, name, (self: any, args: any[]) => {
return {
name: 'fs.' + name,
args: args,
cbIdx: args.length > 0 ? args.length - 1 : -1,
target: self
};
});
TO_PATCH_MACROTASK_METHODS.filter(name => !!fs[name] && typeof fs[name] === 'function')
.forEach(name => {
patchMacroTask(fs, name, (self: any, args: any[]) => {
return {
name: 'fs.' + name,
args: args,
cbIdx: args.length > 0 ? args.length - 1 : -1,
target: self
};
});
});

const realpathOriginalDelegate = fs.realpath?.[api.symbol('OriginalDelegate')];
// This is the only specific method that should be additionally patched because the previous
// `patchMacroTask` has overridden the `realpath` function and its `native` property.
if (realpathOriginalDelegate?.native) {
fs.realpath.native = realpathOriginalDelegate.native;
patchMacroTask(fs.realpath, 'native', (self, args) => ({
args,
target: self,
cbIdx: args.length > 0 ? args.length - 1 : -1,
name: 'fs.realpath.native',
}));
}
});
72 changes: 71 additions & 1 deletion packages/zone.js/test/node/fs.spec.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {closeSync, exists, fstatSync, openSync, read, unlink, unlinkSync, unwatchFile, watch, watchFile, write, writeFile} from 'fs';
import {closeSync, exists, fstatSync, openSync, read, realpath, unlink, unlinkSync, unwatchFile, watch, watchFile, write, writeFile} from 'fs';
import url from 'url';
import util from 'util';

Expand Down Expand Up @@ -41,6 +41,50 @@ describe('nodejs file system', () => {
});
});
});

it('has patched realpath as macroTask', (done) => {
const testZoneSpec = {
name: 'test',
onScheduleTask: (delegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task):
Task => {
return delegate.scheduleTask(targetZone, task);
}
};
const testZone = Zone.current.fork(testZoneSpec);
spyOn(testZoneSpec, 'onScheduleTask').and.callThrough();
testZone.run(() => {
realpath('testfile', () => {
expect(Zone.current).toBe(testZone);
expect(testZoneSpec.onScheduleTask).toHaveBeenCalled();
done();
});
});
});

// https://github.com/angular/angular/issues/45546
// Note that this is intentionally marked with `xit` because `realpath.native`
// is patched by Bazel's `node_patches.js` and doesn't allow further patching
// of `realpath.native` in unit tests. Essentially, there's no original delegate
// for `realpath` because it's also patched. The code below functions correctly
// in the actual production environment.
xit('has patched realpath.native as macroTask', (done) => {
const testZoneSpec = {
name: 'test',
onScheduleTask: (delegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task):
Task => {
return delegate.scheduleTask(targetZone, task);
}
};
const testZone = Zone.current.fork(testZoneSpec);
spyOn(testZoneSpec, 'onScheduleTask').and.callThrough();
testZone.run(() => {
realpath.native('testfile', () => {
expect(Zone.current).toBe(testZone);
expect(testZoneSpec.onScheduleTask).toHaveBeenCalled();
done();
});
});
});
});

describe('watcher related methods test', () => {
Expand Down Expand Up @@ -108,6 +152,32 @@ describe('util.promisify', () => {
});
});

it('fs.realpath should work with util.promisify', (done: DoneFn) => {
const promisifyRealpath = util.promisify(realpath);
promisifyRealpath(currentFile)
.then(
r => {
expect(r).toBeDefined();
done();
},
err => {
fail(`should not be here with error: ${err}`);
});
});

it('fs.realpath.native should work with util.promisify', (done: DoneFn) => {
const promisifyRealpathNative = util.promisify(realpath.native);
promisifyRealpathNative(currentFile)
.then(
r => {
expect(r).toBeDefined();
done();
},
err => {
fail(`should not be here with error: ${err}`);
});
});

it('fs.read should work with util.promisify', (done: DoneFn) => {
const promisifyRead = util.promisify(read);
const fd = openSync(currentFile, 'r');
Expand Down

0 comments on commit 4297005

Please sign in to comment.