Repository: rollup/rollup
File: src/ast/nodes/ImportExpression.ts (TypeScript)

Code before:
			) {
				return;
			}

			// Side-effect only: import('bar').then(() => {})
			if (thenCallback.params.length === 0) {
				return EMPTY_ARRAY;
			}

			const declaration = thenCallback.params[0];
			if (thenCallback.params.length === 1 && declaration instanceof ObjectPattern) {
				return getDeterministicObjectDestructure(declaration);
			}

			return this.hasUnknownAccessedKey ? undefined : [...this.accessedPropKey];
		}
	}

	hasEffects(): boolean {
		return true;
	}

	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
		if (!this.included) {
			this.includeNode();
			if (this.shouldIncludeDynamicAttributes) this.options?.includePath(UNKNOWN_PATH, context);
		}
		this.source.include(context, includeChildrenRecursively);
		if (this.shouldIncludeDynamicAttributes)
			this.options?.include(context, includeChildrenRecursively);
	}

	includeNode() {
		this.included = true;
		this.scope.context.includeDynamicImport(this);
		this.scope.addAccessedDynamicImport(this);
	}

	includePath(path: ObjectPath): void {
		if (!this.included) this.includeNode();
		// Technically, this is not correct as dynamic imports return a Promise.
		if (this.hasUnknownAccessedKey) return;
		if (path[0] === UnknownKey) {
			this.hasUnknownAccessedKey = true;
		} else if (typeof path[0] === 'string') {
			this.accessedPropKey.add(path[0]);
		}
		// Update included paths
		this.scope.context.includeDynamicImport(this);
	}


Review comment (performance):
This should really be moved into `includeNode` because there is no guarantee whether `include` or `includePath` are called first, and either will call `includeNode`. So `includeNode` is the place for anything that should happen once on inclusion, and any mechanism that can include a Node at least needs to make sure that `includeNode` is called if the Node was not included before.

The reason I split up `include`, `includePath` and `includeNode` is that
- `include` is the basic building block of the tree-shaking process. Once a Node is included, it is called again on every tree-shaking pass to see if there were non-included children that are now to be included as well. It can be quite fast because it is only concerned with including the current AST node and its children and not with referenced variables.
- Of course, referenced variables need to be included as well, but they only need to be included once. Then on the next pass, all locations that reference them will be "properly included" by the regular `include` call. The same goes for referenced fields of referenced variables. This is done via `includePath`.
- `includePath` is following references, hence it can be much slower. So if I referenced `foo.bar.baz`, then we run an `includePath` of `["bar", "baz"]` on the variable `foo`, which in turn will trigger an `includePath` of `["baz"]` on whatever is `foo.bar`. In theory, much longer chains are possible. Hence we take care to only do this on the first inclusion of a Node.
- On the other hand due to how this works, we cannot guarantee whether for any Node, `include` or `includePath` are called first. But `includePath` of course should also mark a Node as included. Hence `includeNode` was extracted for things that should happen only once on inclusion.

Code after:
			) {
				return;
			}

			// Side-effect only: import('bar').then(() => {})
			if (thenCallback.params.length === 0) {
				return EMPTY_ARRAY;
			}

			const declaration = thenCallback.params[0];
			if (thenCallback.params.length === 1 && declaration instanceof ObjectPattern) {
				return getDeterministicObjectDestructure(declaration);
			}

			return this.hasUnknownAccessedKey ? undefined : [...this.accessedPropKey];
		}
	}

	hasEffects(): boolean {
		return true;
	}

	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
		if (!this.included) this.includeNode(context);
		this.source.include(context, includeChildrenRecursively);
		if (this.shouldIncludeDynamicAttributes)
			this.options?.include(context, includeChildrenRecursively);
	}

	includeNode(context: InclusionContext) {
		this.included = true;
		if (this.shouldIncludeDynamicAttributes) this.options?.includePath(UNKNOWN_PATH, context);
		this.scope.context.includeDynamicImport(this);
		this.scope.addAccessedDynamicImport(this);
	}

	includePath(path: ObjectPath, context: InclusionContext): void {
		if (!this.included) this.includeNode(context);
		// Technically, this is not correct as dynamic imports return a Promise.
		if (this.hasUnknownAccessedKey) return;
		if (path[0] === UnknownKey) {
			this.hasUnknownAccessedKey = true;
		} else if (typeof path[0] === 'string') {
			this.accessedPropKey.add(path[0]);
		}
		// Update included paths
		this.scope.context.includeDynamicImport(this);
	}

	initialise(): void {
		super.initialise();