Skip to content

Commit

Permalink
Simplify the gencode for idom ve logging when the logOnly parameter…
Browse files Browse the repository at this point in the history
… is used.

Move the logic of whether to construct a `NullRenderer` into the `enter` functions, now the gencode just needs to capture the return value when passing a logonly parameter.

This saves 2 `if` statements and one local variable per `velog` with a logonly parameter.  This also fixes a more basic issue where we would accidentally generate the logic for the logonly expression twice!.

PiperOrigin-RevId: 706049720
  • Loading branch information
lukesandberg authored and copybara-github committed Dec 14, 2024
1 parent e35a0aa commit 502ebd0
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,10 @@ Statement generateMsgGroupCode(MsgFallbackGroupNode node) {
if (phNode.getParent() instanceof VeLogNode) {
VeLogNode parent = (VeLogNode) phNode.getParent();
if (parent.getChild(0) == phNode) {
GenIncrementalDomTemplateBodyVisitor.VeLogStateHolder state =
idomTemplateBodyVisitor.openVeLogNode(parent);
// It is a compiler failure to have a logOnly in a message node.
Preconditions.checkState(state.logOnlyConditional == null);
value = Statements.of(state.enterStatement, value);
value = Statements.of(idomTemplateBodyVisitor.openVeLogNode(parent), value);
}
if (parent.getChild(parent.numChildren() - 1) == phNode) {
value = Statements.of(value, idomTemplateBodyVisitor.exitVeLogNode(parent, null));
value = Statements.of(value, idomTemplateBodyVisitor.exitVeLogNode(parent));
}
}
switchBuilder.addCase(Expressions.stringLiteral(ph.getKey()), value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_PUSH_KEY;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_PUSH_MANUAL_KEY;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_TEXT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_TODEFAULT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_TONULL;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_VERIFY_LOGONLY;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_VISIT_HTML_COMMENT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.SOY_IDOM_CALL_DYNAMIC_ATTRIBUTES;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.SOY_IDOM_CALL_DYNAMIC_CSS;
Expand Down Expand Up @@ -139,33 +136,6 @@
* print the function calls and changing how statements are combined.
*/
public final class GenIncrementalDomTemplateBodyVisitor extends GenJsTemplateBodyVisitor {

/**
* Class that contains the state generated from visiting the beginning of a velogging statement.
* This allows one to generate code such as
*
* <pre>
* var velog_1 = foobar;
* if (velog_1) {
* idom = idom.toNullRenderer();
* }
* idom.enter(new Metadata(...));
* ...
* if (velog_1) {
* idom = idom.toDefaultRenderer();
* }
* </pre>
*/
static class VeLogStateHolder {
Expression logOnlyConditional; // Holds the variable reference to velog_1
Statement enterStatement; // Contains the idom.enter(...) statement

public VeLogStateHolder(Expression logOnlyConditional, Statement enterStatement) {
this.logOnlyConditional = logOnlyConditional;
this.enterStatement = enterStatement;
}
}

private final Deque<SanitizedContentKind> contentKind;
private final List<Statement> staticVarDeclarations;
private final boolean generatePositionalParamsSignature;
Expand Down Expand Up @@ -1080,45 +1050,29 @@ protected Statement visitSkipNode(SkipNode node) {
@Override
protected Statement visitVeLogNode(VeLogNode node) {
List<Statement> statements = new ArrayList<>();
VeLogStateHolder state = openVeLogNode(node);
statements.add(state.enterStatement);
statements.add(openVeLogNode(node));
statements.addAll(visitChildren(node));
statements.add(exitVeLogNode(node, state.logOnlyConditional));
statements.add(exitVeLogNode(node));
return Statements.of(statements);
}

VeLogStateHolder openVeLogNode(VeLogNode node) {
Expression isLogOnly = Expressions.LITERAL_FALSE;
VariableDeclaration isLogOnlyVar;
Expression isLogOnlyReference = null;
List<Statement> stmts = new ArrayList<>();
Statement openVeLogNode(VeLogNode node) {
Expression veData = getExprTranslator().exec(node.getVeDataExpression());
if (node.getLogonlyExpression() != null) {
String idName = "velog_" + staticsCounter++;
isLogOnlyReference = id(idName);
isLogOnly = getExprTranslator().exec(node.getLogonlyExpression());
isLogOnlyVar = VariableDeclaration.builder(idName).setRhs(isLogOnly).build();
stmts.add(isLogOnlyVar);
stmts.add(
Statements.ifStatement(
INCREMENTAL_DOM_VERIFY_LOGONLY.call(isLogOnlyVar.ref()),
Statements.assign(INCREMENTAL_DOM, INCREMENTAL_DOM_TONULL.call()))
.build());
return Statements.assign(
INCREMENTAL_DOM,
INCREMENTAL_DOM_ENTER.call(
veData, getExprTranslator().exec(node.getLogonlyExpression())));
}
Expression veData = getExprTranslator().exec(node.getVeDataExpression());
stmts.add(INCREMENTAL_DOM_ENTER.call(veData, isLogOnly).asStatement());
return new VeLogStateHolder(isLogOnlyReference, Statements.of(stmts));
return INCREMENTAL_DOM_ENTER.call(veData).asStatement();
}

Statement exitVeLogNode(VeLogNode node, @Nullable Expression isLogOnly) {
Statement exit = INCREMENTAL_DOM_EXIT.call().asStatement();
if (isLogOnly != null) {
return Statements.of(
exit,
Statements.ifStatement(
isLogOnly, Statements.assign(INCREMENTAL_DOM, INCREMENTAL_DOM_TODEFAULT.call()))
.build());
Statement exitVeLogNode(VeLogNode node) {
var exit = INCREMENTAL_DOM_EXIT.call();
if (node.getLogonlyExpression() != null) {
return Statements.assign(INCREMENTAL_DOM, exit);
}
return exit;
return exit.asStatement();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,6 @@ final class IncrementalDomRuntime {

public static final Expression INCREMENTAL_DOM_EXIT = INCREMENTAL_DOM.dotAccess("exit");

public static final Expression INCREMENTAL_DOM_VERIFY_LOGONLY =
INCREMENTAL_DOM.dotAccess("verifyLogOnly");

public static final Expression INCREMENTAL_DOM_TODEFAULT =
INCREMENTAL_DOM.dotAccess("toDefaultRenderer");

public static final Expression INCREMENTAL_DOM_TONULL =
INCREMENTAL_DOM.dotAccess("toNullRenderer");

public static final Expression INCREMENTAL_DOM_TEXT = INCREMENTAL_DOM.dotAccess("text");
public static final Expression INCREMENTAL_DOM_PRINT = INCREMENTAL_DOM.dotAccess("print");
public static final Expression INCREMENTAL_DOM_VISIT_HTML_COMMENT =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ private static LoggingFunctionInfo getLoggingFunctionInfo(CallParamNode paramNod
/**
* Element composition calls are deconstructed into call nodes. However, some of the attributes
* contain velogging functions. This takes those attributes and puts them on a wrapping `veAttr`
* element, which the runtime libarary then manages. So the overall DOM structure becomes
* element, which the runtime library then manages. So the overall DOM structure becomes
*
* <pre>{@code
* <velog>
Expand Down
117 changes: 47 additions & 70 deletions javascript/api_idom.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/**
* @fileoverview
*
* Functions necessary to interact with the Soy-Idom runtime.
Expand Down Expand Up @@ -113,13 +112,10 @@ export interface IncrementalDomRenderer {
skipNode(): void;
applyAttrs(): void;
applyStatics(statics: incrementaldom.Statics): void;
enter(veData: $$VisualElementData, logOnly: boolean): void;
exit(): void;
toNullRenderer(): IncrementalDomRenderer;
toDefaultRenderer(): IncrementalDomRenderer;
setLogger(logger: Logger | null): void;
getLogger(): Logger | null;
verifyLogOnly(logOnly: boolean): boolean;
enter(veData: $$VisualElementData, logOnly?: boolean): IncrementalDomRenderer;
exit(): IncrementalDomRenderer;
setLogger(logger: Logger | undefined): void;
getLogger(): Logger | undefined;
evalLoggingFunction(
name: string,
args: Array<{}>,
Expand Down Expand Up @@ -159,7 +155,7 @@ export class IncrementalDomRendererImpl implements IncrementalDomRenderer {
// Note that for performance, the "stack" is implemented as a string with
// the items being `${SIZE OF KEY}${DELIMITER}${KEY}`.
private readonly keyStackHolder: string[] = [];
private logger: Logger | null = null;
private logger: Logger | undefined;

/**
* Pushes/pops the given key from `keyStack` (versus `Array#concat`)
Expand Down Expand Up @@ -436,60 +432,44 @@ export class IncrementalDomRendererImpl implements IncrementalDomRenderer {
/**
* Called when a `{velog}` statement is entered.
*/
enter(veData: $$VisualElementData, logOnly: boolean) {
if (this.logger) {
this.logger.enter(
new ElementMetadata(veData.getVe().getId(), veData.getData(), logOnly),
enter(
veData: $$VisualElementData,
logOnly?: boolean,
): IncrementalDomRenderer {
const logger = this.logger;
if (logger) {
logger.enter(
new ElementMetadata(
veData.getVe().getId(),
veData.getData(),
!!logOnly,
),
);
} else if (logOnly) {
throw new Error(
'Cannot set logonly="true" unless there is a logger configured',
);
}
return logOnly ? new NullRenderer(this) : this;
}

/**
* Called when a `{velog}` statement is exited.
*/
exit() {
if (this.logger) {
this.logger.exit();
}
}

/**
* Switches runtime to produce incremental dom calls that do not traverse
* the DOM. This happens when logOnly in a velogging node is set to true.
*/
toNullRenderer(): IncrementalDomRenderer {
const nullRenderer = new NullRenderer(this);
return nullRenderer;
}

toDefaultRenderer(): IncrementalDomRenderer {
throw new Error(
'Cannot transition a default renderer to a default renderer',
);
exit(): IncrementalDomRenderer {
this.logger?.exit();
return this;
}

/** Called by user code to configure logging */
setLogger(logger: Logger | null) {
setLogger(logger: Logger | undefined) {
this.logger = logger;
}

getLogger() {
return this.logger;
}

/**
* Used to trigger the requirement that logOnly can only be true when a
* logger is configured. Otherwise, it is a passthrough function.
*/
verifyLogOnly(logOnly: boolean) {
if (!this.logger && logOnly) {
throw new Error(
'Cannot set logonly="true" unless there is a logger configured',
);
}
return logOnly;
}

/*
* Called when a logging function is evaluated.
*/
Expand Down Expand Up @@ -640,8 +620,8 @@ export class NullRenderer extends IncrementalDomRendererImpl {
override skipNode() {}

/** Returns to the default renderer which will traverse the DOM. */
override toDefaultRenderer() {
this.renderer.setLogger(this.getLogger());
override exit(): IncrementalDomRenderer {
super.exit();
return this.renderer;
}
}
Expand Down Expand Up @@ -729,15 +709,17 @@ export class FalsinessRenderer extends IncrementalDomRendererImpl {
override popManualKey(): void {}
override pushKey(key: string): void {}
override popKey(): void {}
override enter(): void {}
override exit(): void {}
override setLogger(logger: Logger | null): void {}
override getLogger(): Logger | null {
return null;
override enter(): IncrementalDomRenderer {
return this;
}
override exit(): IncrementalDomRenderer {
return this;
}
override verifyLogOnly(logOnly: boolean): boolean {
return logOnly;
override setLogger(logger: Logger | undefined): void {}
override getLogger(): Logger | undefined {
return undefined;
}

override evalLoggingFunction(
name: string,
args: Array<{}>,
Expand Down Expand Up @@ -885,35 +867,30 @@ export class BufferingIncrementalDomRenderer implements IncrementalDomRenderer {
popKey(): void {
this.buffer.push(noArgCallConsts.popKey);
}
enter(veData: $$VisualElementData, logOnly: boolean): void {
enter(
veData: $$VisualElementData,
logOnly?: boolean,
): IncrementalDomRenderer {
this.buffer.push((actual) => {
actual.enter(veData, logOnly);
});
return logOnly ? new NullRenderer(this) : this;
}
exit(): void {
exit(): IncrementalDomRenderer {
this.buffer.push(noArgCallConsts.exit);
return this;
}
toNullRenderer(): IncrementalDomRenderer {
return new NullRenderer(this);
}
toDefaultRenderer(): IncrementalDomRenderer {
throw new Error(
'Cannot transition a buffered renderer to a default renderer',
);
}
setLogger(logger: Logger | null): void {

setLogger(logger: Logger | undefined): void {
throw new Error(
'Tried to call setLogger on BufferingIncrementalDomRenderer.',
);
}
getLogger(): Logger | null {
getLogger(): Logger | undefined {
throw new Error(
'Tried to call getLogger on BufferingIncrementalDomRenderer.',
);
}
verifyLogOnly(logOnly: boolean): boolean {
return logOnly;
}
evalLoggingFunction(
name: string,
args: Array<{}>,
Expand Down
4 changes: 2 additions & 2 deletions javascript/element_lib_idom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export abstract class SoyElement<TData extends {} | null, TInterface extends {}>
private patchHandler: ((prev: TInterface, next: TInterface) => void) | null =
null;
private syncState = true;
private loggerPrivate: Logger | null = null;
private loggerPrivate: Logger | undefined;
// Marker so that future element accesses can find this Soy element from the
// DOM
key: string = '';
Expand All @@ -79,7 +79,7 @@ export abstract class SoyElement<TData extends {} | null, TInterface extends {}>
* is called with a Renderer that has its own Logger, Renderer's Logger is
* used instead.
*/
setLogger(logger: Logger | null): this {
setLogger(logger: Logger | undefined): this {
this.loggerPrivate = logger;
return this;
}
Expand Down

0 comments on commit 502ebd0

Please sign in to comment.