bug: zoneWrap applies broken arguments after bundled

[lacolaco]

Version info

Angular: v13

Firebase: 9.6

AngularFire: 7.2.0

Other (e.g. Ionic/Cordova, Node, browser, operating system):

How to reproduce these conditions

• Look at the distributed package at https://unpkg.com/@angular/[email protected]/bundles/angular-fire.umd.js

Actual behavior

angularfire/src/zones.ts

Lines 162 to 175 in b60670c

	return function() {
	let macrotask: MacroTask | undefined;
	// if this is a callback function, e.g, onSnapshot, we should create a microtask and invoke it
	// only once one of the callback functions is tripped.
	for (let i = 0; i < arguments.length; i++) {
	if (typeof arguments[i] === 'function') {
	if (blockUntilFirst) {
	macrotask ||= run(() => Zone.current.scheduleMacroTask('firebaseZoneBlock', noop, {}, noop, noop));
	}
	// TODO create a microtask to track callback functions
	arguments[i] = zoneWrapFn(arguments[i], macrotask);
	}
	}
	const ret = runOutsideAngular(() => (it as any).apply(this, arguments));

  return function() {
    let macrotask: MacroTask | undefined;
    // if this is a callback function, e.g, onSnapshot, we should create a microtask and invoke it
    // only once one of the callback functions is tripped.
    for (let i = 0; i < arguments.length; i++) {
      if (typeof arguments[i] === 'function') {
        if (blockUntilFirst) {
          macrotask ||= run(() => Zone.current.scheduleMacroTask('firebaseZoneBlock', noop, {}, noop, noop));
        }
        // TODO create a microtask to track callback functions
        arguments[i] = zoneWrapFn(arguments[i], macrotask);
      }
    }
    const ret = runOutsideAngular(() => (it as any).apply(this, arguments)); // <--- `arguments` of the top function

In this source file, arguments is used as an array that contains the arguments of the function at the beginning. In fact, this code works fine in builds targeting ES2015 and later.

If you look at the zoneWrap part of the UNPKG file, you will see that the callback for the runOutsideAngular function is transpiled to a function, so the internal arguments changes the arguments it refers to.

https://unpkg.com/@angular/[email protected]/bundles/angular-fire.umd.js

        return function () {
            var _this_1 = this;
            var macrotask;
            // if this is a callback function, e.g, onSnapshot, we should create a microtask and invoke it
            // only once one of the callback functions is tripped.
            for (var i = 0; i < arguments.length; i++) {
                if (typeof arguments[i] === 'function') {
                    if (blockUntilFirst) {
                        macrotask || (macrotask = run(function () { return Zone.current.scheduleMacroTask('firebaseZoneBlock', noop, {}, noop, noop); }));
                    }
                    // TODO create a microtask to track callback functions
                    arguments[i] = zoneWrapFn(arguments[i], macrotask);
                }
            }
            var ret = runOutsideAngular(function () { return it.apply(_this_1, arguments); });  // <--- `arguments` of the nested function

How to fix

1. Upgrade ng-packagr to v13 and stop shiping UMD bundle (no ES5 transpliation)
2. Stop direct referring to arguments in callback functions (fix(core): avoid referring to arguments in arrow functions #3127)
3. Use (...args) => {}

[google-oss-bot]

This issue does not seem to follow the issue template. Make sure you provide all the required information.

[jamesdaniels]

Cutting 7.2.1 with the fix now