Correctly handle side effects when a namespace is nested in an object (…

…#4758)

src/ast/variables/NamespaceVariable.ts

@@ -3,12 +3,15 @@ import type { AstContext } from '../../Module';
 import { getToStringTagValue, MERGE_NAMESPACES_VARIABLE } from '../../utils/interopHelpers';
 import type { RenderOptions } from '../../utils/renderHelpers';
 import { getSystemExportStatement } from '../../utils/systemJsRendering';
+import type { HasEffectsContext } from '../ExecutionContext';
+import { INTERACTION_ASSIGNED, INTERACTION_CALLED } from '../NodeInteractions';
+import type { NodeInteraction, NodeInteractionWithThisArgument } from '../NodeInteractions';
 import type Identifier from '../nodes/Identifier';
 import type { LiteralValueOrUnknown } from '../nodes/shared/Expression';
 import { UnknownValue } from '../nodes/shared/Expression';
 import type ChildScope from '../scopes/ChildScope';
-import type { ObjectPath } from '../utils/PathTracker';
-import { SymbolToStringTag } from '../utils/PathTracker';
+import type { ObjectPath, PathTracker } from '../utils/PathTracker';
+import { SymbolToStringTag, UNKNOWN_PATH } from '../utils/PathTracker';
 import Variable from './Variable';
 
 export default class NamespaceVariable extends Variable {
@@ -32,6 +35,34 @@ export default class NamespaceVariable extends Variable {
 		this.name = identifier.name;
 	}
 
+	deoptimizePath(path: ObjectPath) {
+		if (path.length > 1) {
+			const key = path[0];
+			if (typeof key === 'string') {
+				this.getMemberVariables()[key]?.deoptimizePath(path.slice(1));
+			}
+		}
+	}
+
+	deoptimizeThisOnInteractionAtPath(
+		interaction: NodeInteractionWithThisArgument,
+		path: ObjectPath,
+		recursionTracker: PathTracker
+	) {
+		if (path.length > 1 || (path.length === 1 && interaction.type === INTERACTION_CALLED)) {
+			const key = path[0];
+			if (typeof key === 'string') {
+				this.getMemberVariables()[key]?.deoptimizeThisOnInteractionAtPath(
+					interaction,
+					path.slice(1),
+					recursionTracker
+				);
+			} else {
+				interaction.thisArg.deoptimizePath(UNKNOWN_PATH);
+			}
+		}
+	}
+
 	getLiteralValueAtPath(path: ObjectPath): LiteralValueOrUnknown {
 		if (path[0] === SymbolToStringTag) {
 			return 'Module';
@@ -55,8 +86,28 @@ export default class NamespaceVariable extends Variable {
 		return (this.memberVariables = memberVariables);
 	}
 
-	hasEffectsOnInteractionAtPath(): boolean {
-		return false;
+	hasEffectsOnInteractionAtPath(
+		path: ObjectPath,
+		interaction: NodeInteraction,
+		context: HasEffectsContext
+	): boolean {
+		const { type } = interaction;
+		if (path.length === 0) {
+			// This can only be a call anyway
+			return true;
+		}
+		if (path.length === 1 && type !== INTERACTION_CALLED) {
+			return type === INTERACTION_ASSIGNED;
+		}
+		const key = path[0];
+		if (typeof key !== 'string') {
+			return true;
+		}
+		const memberVariable = this.getMemberVariables()[key];
+		return (
+			!memberVariable ||
+			memberVariable.hasEffectsOnInteractionAtPath(path.slice(1), interaction, context)
+		);
 	}
 
 	include(): void {

test/function/samples/namespace-member-side-effects/assignment/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+	description: 'checks side effects when reassigning namespace members',
+	options: {
+		treeshake: { tryCatchDeoptimization: false }
+	}
+};

test/function/samples/namespace-member-side-effects/assignment/api/index.js

@@ -0,0 +1,5 @@
+import * as namespace from './namespace.js';
+
+export default {
+	namespace
+};

test/function/samples/namespace-member-side-effects/assignment/main.js

@@ -0,0 +1,9 @@
+import api from './api/index';
+
+let errored = false;
+try {
+	api.namespace.x = 1;
+} catch {
+	errored = true;
+}
+assert.ok(errored, 'namespace assignment should be preserved');

test/function/samples/namespace-member-side-effects/call/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+	description: 'checks side effects when calling a namespace',
+	options: {
+		treeshake: { tryCatchDeoptimization: false }
+	}
+};

test/function/samples/namespace-member-side-effects/call/api/index.js

@@ -0,0 +1,5 @@
+import * as namespace from './namespace.js';
+
+export default {
+	namespace
+};

test/function/samples/namespace-member-side-effects/call/main.js

@@ -0,0 +1,9 @@
+import api from './api/index';
+
+let errored = false
+try {
+  api.namespace()
+} catch {
+  errored = true;
+}
+assert.ok(errored, 'namespace call should be preserved')

test/function/samples/namespace-member-side-effects/member-call/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'respects side effects when namespace members are called'
+};

test/function/samples/namespace-member-side-effects/member-call/api/index.js

@@ -0,0 +1,5 @@
+import * as namespace from './namespace.js';
+
+export default {
+	namespace
+};

test/function/samples/namespace-member-side-effects/member-call/api/namespace.js

@@ -0,0 +1,5 @@
+import { sideEffects } from './sideEffects';
+
+export function sideEffectFunction() {
+	sideEffects.push('fn called');
+}

test/function/samples/namespace-member-side-effects/member-call/api/sideEffects.js

@@ -0,0 +1 @@
+export const sideEffects = [];

test/function/samples/namespace-member-side-effects/member-call/main.js

@@ -0,0 +1,6 @@
+import api from './api/index.js';
+import { sideEffects } from './api/sideEffects';
+
+api.namespace.sideEffectFunction();
+
+assert.deepStrictEqual(sideEffects, ['fn called']);

test/function/samples/namespace-member-side-effects/missing-access/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+	description: 'respects side effects when accessing missing namespace members',
+	options: {
+		treeshake: { tryCatchDeoptimization: false }
+	}
+};

test/function/samples/namespace-member-side-effects/missing-access/api/index.js

@@ -0,0 +1,5 @@
+import * as namespace from './namespace.js';
+
+export default {
+	namespace
+};

test/function/samples/namespace-member-side-effects/missing-access/main.js

@@ -0,0 +1,9 @@
+import api from './api/index';
+
+let errored = false;
+try {
+	api.namespace.missing.foo;
+} catch {
+	errored = true;
+}
+assert.ok(errored, 'unknown nested member access should be preserved');

test/function/samples/namespace-member-side-effects/unknown-access/_config.js

@@ -0,0 +1,7 @@
+module.exports = {
+	description: 'respects side effects when accessing unknown namespace members',
+	options: {
+		external: ['external'],
+		treeshake: { tryCatchDeoptimization: false }
+	}
+};

test/function/samples/namespace-member-side-effects/unknown-access/api/index.js

@@ -0,0 +1,5 @@
+import * as namespace from './namespace.js';
+
+export default {
+	namespace
+};

test/function/samples/namespace-member-side-effects/unknown-access/main.js

@@ -0,0 +1,10 @@
+import api from './api/index';
+import { external } from 'external';
+
+let errored = false;
+try {
+	api.namespace[external].foo;
+} catch {
+	errored = true;
+}
+assert.ok(errored, 'unknown nested member access should be preserved');