Skip to content

feat(tab): modular ionic migration and lazy-load specs #31115

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.

Sign up for GitHub

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

Open
thetaPC wants to merge 4 commits into
base: ionic-modular
Choose a base branch
from
Open

feat(tab): modular ionic migration and lazy-load specs #31115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
060dc9b
feat(tab): modular ionic migration and lazy-load specsx
thetaPC May 5, 2026
cb81ec0
chore(): run build
thetaPC May 5, 2026
575d355
docs(tab): add comment regarding lock bug
thetaPC May 11, 2026
d0c781b
Merge branch 'ionic-modular' of github.com:ionic-team/ionic-framework…
thetaPC May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion core/api.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2621,7 +2621,6 @@ ion-tab,shadow
ion-tab,prop,component,Function | HTMLElement | null | string | undefined,undefined,false,false
ion-tab,prop,mode,"ios" | "md",undefined,false,false
ion-tab,prop,tab,string,undefined,true,false
ion-tab,prop,theme,"ios" | "md" | "ionic",undefined,false,false
ion-tab,method,setActive,setActive() => Promise<void>

ion-tab-bar,shadow
Expand Down
8 changes: 0 additions & 8 deletions core/src/components.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3961,10 +3961,6 @@ export namespace Components {
* A tab id must be provided for each `ion-tab`. It's used internally to reference the selected tab or by the router to switch between them.
*/
"tab": string;
/**
* The theme determines the visual appearance of the component.
*/
"theme"?: "ios" | "md" | "ionic";
}
interface IonTabBar {
/**
Expand Down Expand Up @@ -10051,10 +10047,6 @@ declare namespace LocalJSX {
* A tab id must be provided for each `ion-tab`. It's used internally to reference the selected tab or by the router to switch between them.
*/
"tab": string;
/**
* The theme determines the visual appearance of the component.
*/
"theme"?: "ios" | "md" | "ionic";
}
interface IonTabBar {
/**
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/tab/tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type { ComponentRef, FrameworkDelegate } from '../../interface';

/**
* @virtualProp {"ios" | "md"} mode - The mode determines the platform behaviors of the component.
* @virtualProp {"ios" | "md" | "ionic"} theme - The theme determines the visual appearance of the component.
*/
@Component({
tag: 'ion-tab',
Expand Down Expand Up @@ -65,6 +64,7 @@ export class Tab implements ComponentInterface {
}
}

// TODO(FW-7296): Failed first attach locks the tab forever — needs async/await + retry on rejection.
private prepareLazyLoaded(): Promise<HTMLElement | undefined> {
if (!this.loaded && this.component != null) {
this.loaded = true;
Expand Down
44 changes: 44 additions & 0 deletions core/src/components/tab/test/tab.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { newSpecPage } from '@stencil/core/testing';

import type { FrameworkDelegate } from '../../../interface';
import { Tab } from '../tab';

const mockDelegate = (attachViewToDom: FrameworkDelegate['attachViewToDom']): FrameworkDelegate => ({
attachViewToDom,
removeViewFromDom: jest.fn().mockResolvedValue(undefined),
});

describe('ion-tab: lazy loading', () => {
it('should attach the component only once across multiple setActive calls', async () => {
const page = await newSpecPage({
components: [Tab],
html: '<ion-tab tab="home" component="ion-content"></ion-tab>',
});

const tabEl = page.body.querySelector('ion-tab') as any;
const attachViewToDom = jest.fn().mockResolvedValue(document.createElement('div'));
tabEl.delegate = mockDelegate(attachViewToDom);

await tabEl.setActive();
await tabEl.setActive();

expect(attachViewToDom).toHaveBeenCalledTimes(1);
});

// TODO(FW-7296): Flip to assert retry once the failed-attach lockout is fixed
it('documents current behavior: a failed first attach is not retried', async () => {
const page = await newSpecPage({
components: [Tab],
html: '<ion-tab tab="home" component="ion-content"></ion-tab>',
});

const tabEl = page.body.querySelector('ion-tab') as any;
const attachViewToDom = jest.fn().mockRejectedValue(new Error('attach failed'));
tabEl.delegate = mockDelegate(attachViewToDom);

await expect(tabEl.setActive()).rejects.toThrow('attach failed');
await tabEl.setActive();

expect(attachViewToDom).toHaveBeenCalledTimes(1);
});
Comment on lines +27 to +43
Copy link
Copy Markdown
Member

@ShaneK ShaneK May 11, 2026

Choose a reason for hiding this comment

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

Is the "permanent failure" behavior actually what we want here, or just what the code currently does? The synchronous try/catch in prepareLazyLoaded can't catch the async rejection from attachComponent, and this.loaded = true is set before the await, so any first-attempt failure locks the tab out for good. A passing test on this makes a future fix (resetting loaded on rejection, or moving the catch around an await) look like a regression. Worth either filing a follow-up and referencing it here, or renaming the test so it reads "documents current behavior" rather than "asserts a contract"?

Copy link
Copy Markdown
Contributor Author

@thetaPC thetaPC May 11, 2026

Choose a reason for hiding this comment

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

I added TODOs to go back and fix this on a separate ticket: 575d355

});
4 changes: 2 additions & 2 deletions packages/angular/src/directives/proxies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2347,15 +2347,15 @@ export declare interface IonSplitPane extends Components.IonSplitPane {


@ProxyCmp({
inputs: ['component', 'mode', 'tab', 'theme'],
inputs: ['component', 'mode', 'tab'],
methods: ['setActive']
})
@Component({
selector: 'ion-tab',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['component', 'mode', 'tab', 'theme'],
inputs: ['component', 'mode', 'tab'],
})
export class IonTab {
protected el: HTMLIonTabElement;
Expand Down
4 changes: 2 additions & 2 deletions packages/angular/standalone/src/directives/proxies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2088,15 +2088,15 @@ export declare interface IonSplitPane extends Components.IonSplitPane {

@ProxyCmp({
defineCustomElementFn: defineIonTab,
inputs: ['component', 'mode', 'tab', 'theme'],
inputs: ['component', 'mode', 'tab'],
methods: ['setActive']
})
@Component({
selector: 'ion-tab',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['component', 'mode', 'tab', 'theme'],
inputs: ['component', 'mode', 'tab'],
standalone: true
})
export class IonTab {
Expand Down
Loading