Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): attempt to recover from user errors during creation #36381

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -30,7 +30,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 137320,
"main-es2015": 137897,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is mostly accumulation from other PRs? The change itself doesn't seem to merit this jump in size.

"polyfills-es2015": 37334
}
}
Expand All @@ -49,7 +49,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 226728,
"main-es2015": 227258,
"polyfills-es2015": 36657,
"5-es2015": 779
}
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/render3/instructions/lview_debug.ts
Expand Up @@ -143,6 +143,7 @@ export const TViewConstructor = class TView implements ITView {
public firstChild: ITNode|null, //
public schemas: SchemaMetadata[]|null, //
public consts: TConstants|null, //
public incompleteFirstPass: boolean //
) {}

get template_(): string {
Expand Down
28 changes: 23 additions & 5 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -374,6 +374,14 @@ export function renderView<T>(tView: TView, lView: LView, context: T): void {
renderChildComponents(lView, components);
}

} catch (error) {
// If we didn't manage to get past the first template pass due to
// an error, mark the view as corrupted so we can try to recover.
if (tView.firstCreatePass) {
tView.incompleteFirstPass = true;
}

throw error;
} finally {
lView[FLAGS] &= ~LViewFlags.CreationMode;
leaveView();
Expand Down Expand Up @@ -598,10 +606,17 @@ export function saveResolvedLocalsInData(
* @returns TView
*/
export function getOrCreateTComponentView(def: ComponentDef<any>): TView {
return def.tView ||
(def.tView = createTView(
TViewType.Component, -1, def.template, def.decls, def.vars, def.directiveDefs,
def.pipeDefs, def.viewQuery, def.schemas, def.consts));
const tView = def.tView;

// Create a TView if there isn't one, or recreate it if the first create pass didn't
// complete successfuly since we can't know for sure whether it's in a usable shape.
if (tView === null || tView.incompleteFirstPass) {
return def.tView = createTView(
TViewType.Component, -1, def.template, def.decls, def.vars, def.directiveDefs,
def.pipeDefs, def.viewQuery, def.schemas, def.consts);
}

return tView;
}


Expand Down Expand Up @@ -662,7 +677,9 @@ export function createTView(
typeof pipes === 'function' ? pipes() : pipes, // pipeRegistry: PipeDefList|null,
null, // firstChild: TNode|null,
schemas, // schemas: SchemaMetadata[]|null,
consts) : // consts: TConstants|null
consts, // consts: TConstants|null
false // incompleteFirstPass: boolean
) :
{
type: type,
id: viewIndex,
Expand Down Expand Up @@ -694,6 +711,7 @@ export function createTView(
firstChild: null,
schemas: schemas,
consts: consts,
incompleteFirstPass: false
};
}

Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/render3/interfaces/view.ts
Expand Up @@ -684,6 +684,12 @@ export interface TView {
* Used for directive matching, attribute bindings, local definitions and more.
*/
consts: TConstants|null;

/**
* Indicates that there was an error before we managed to complete the first create pass of the
* view. This means that the view is likely corrupted and we should try to recover it.
*/
incompleteFirstPass: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of potentially making this a counter so that we can stop trying after a certain number of attempts. I decided to keep it simple for now, but I'm open to changing it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, I think that keeping it simple for now is the way to go 👍

}

export const enum RootContextFlags {
Expand Down