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.

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

refactor(tests): reduce TypeScript errors #10344

Merged
merged 4 commits into from
Sep 24, 2024
Merged

refactor(tests): reduce TypeScript errors #10344

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
2ba7034
refactor(tests): reduce TypeScript errors
maxpatiiuk Sep 18, 2024
9316229
revert: undo loosening CustomEvent type
maxpatiiuk Sep 19, 2024
6e7d00a
refactor(functional): remove unused props
maxpatiiuk Sep 24, 2024
cf7ffb3
fix(tests): correctly call native fetch
maxpatiiuk Sep 24, 2024
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
8 changes: 4 additions & 4 deletions packages/calcite-components/src/components/chip-group/chip-group.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ describe("calcite-chip-group", () => {
expect(chipSelectSpy1).toHaveReceivedEventTimes(0);
expect(chipSelectSpy2).toHaveReceivedEventTimes(0);

await chip5.setAttribute("selected", true);
await chip5.toggleAttribute("selected", true);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a boolean attribute
Before it was being set here to selected="true"

await page.waitForChanges();
expect(chipGroupSelectSpy).toHaveReceivedEventTimes(0);
expect(chipSelectSpy1).toHaveReceivedEventTimes(0);
Expand Down Expand Up @@ -569,15 +569,15 @@ describe("calcite-chip-group", () => {
expect(await element.getProperty("selectedItems")).toHaveLength(0);
await selectedItemAsserter([]);

chip5.setAttribute("selected", true);
chip5.toggleAttribute("selected", true);
await page.waitForChanges();
expect(chipGroupSelectSpy).toHaveReceivedEventTimes(0);
expect(chipSelectSpy1).toHaveReceivedEventTimes(0);
expect(chipSelectSpy2).toHaveReceivedEventTimes(0);
expect(await element.getProperty("selectedItems")).toHaveLength(1);
await selectedItemAsserter([chip5.id]);

chip4.setAttribute("selected", true);
chip4.toggleAttribute("selected", true);
await page.waitForChanges();
expect(chipGroupSelectSpy).toHaveReceivedEventTimes(0);
expect(chipSelectSpy1).toHaveReceivedEventTimes(0);
Expand Down Expand Up @@ -617,7 +617,7 @@ describe("calcite-chip-group", () => {
expect(chipSelectSpy1).toHaveReceivedEventTimes(0);
expect(chipSelectSpy2).toHaveReceivedEventTimes(0);

chip5.setAttribute("selected", true);
chip5.toggleAttribute("selected", true);
await page.waitForChanges();
expect(chipGroupSelectSpy).toHaveReceivedEventTimes(0);
expect(chipSelectSpy1).toHaveReceivedEventTimes(0);
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/chip/chip.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ describe("calcite-chip", () => {
await page.setContent(`<div class="calcite-mode-dark">${chipSnippet}</div>`);

const chipEl = await page.find(`calcite-chip`);
chipEl.setAttribute("closed", true);
chipEl.toggleAttribute("closed", true);
await page.waitForChanges();

expect(await chipEl.isVisible()).toBe(false);
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/combobox/combobox.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1632,7 +1632,7 @@ describe("calcite-combobox", () => {
let element: E2EElement;
let comboboxItem: E2EElement;
let itemNestedLi: E2EElement;
let closeEvent: Promise<void>;
let closeEvent: Promise<unknown>;
Copy link
Member Author

@maxpatiiuk maxpatiiuk Sep 18, 2024 •
edited by jcfranco
Loading

Choose a reason for hiding this comment

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

Stencil's waitForEvent has return type Promise<any> so this worked.
In Lumina, the return type is Promise<SerializedEvent>.

(at runtime, Stencil's waitForEvent also returned Promise, but they didn't publicly expose the SerializedEvent type)


beforeEach(async () => {
page = await newE2EPage();
Expand Down
4 changes: 0 additions & 4 deletions packages/calcite-components/src/components/functional/XButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,13 @@ export const XButton: FunctionalComponent<XButtonProps> = ({
disabled,
key,
label,
onClick,
ref,
scale,
}): VNode => (
<button
aria-label={label}
class={CSS.button}
disabled={disabled}
key={key}
onClick={onClick}
ref={ref}
tabIndex={-1}
type="button"
>
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/list-item-group/list-item-group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class ListItemGroup implements InteractiveComponent {
* Fires when changes occur in the default slot, notifying parent lists of the changes.
*/
@Event({ cancelable: false })
calciteInternalListItemGroupDefaultSlotChange: EventEmitter<DragEvent>;
calciteInternalListItemGroupDefaultSlotChange: EventEmitter<void>;
Copy link
Member Author

Choose a reason for hiding this comment

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

it didn't actually emit this type, and no one was listening for it to emit this type


// --------------------------------------------------------------------------
//
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/list-item/list-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ export class ListItem
//
// --------------------------------------------------------------------------

private dragHandleSelectedChangeHandler = (event: CustomEvent): void => {
private dragHandleSelectedChangeHandler = (event: CustomEvent<void>): void => {
this.dragSelected = (event.target as HTMLCalciteHandleElement).selected;
this.calciteListItemDragHandleChange.emit();
event.stopPropagation();
Expand Down
6 changes: 3 additions & 3 deletions packages/calcite-components/src/components/loader/loader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ export class Loader implements LocalizedComponent {
return (
<Host
aria-label={label}
aria-valuemax={isDeterminate ? 100 : undefined}
aria-valuemin={isDeterminate ? 0 : undefined}
aria-valuenow={isDeterminate ? valueNow : undefined}
aria-valuemax={isDeterminate ? "100" : undefined}
Copy link
Member Author

Choose a reason for hiding this comment

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

Stencil's aria-valuemax type is string | number, and at runtime, the DOM stringifies numbers automatically, so this was good.
However, the TypeScript's type for HTMLElement.ariaValuemax is string. Since this code is converted by the codemod to this.el.ariaValuemax = ..., we need to be setting strings here to avoid type error.

aria-valuemin={isDeterminate ? "0" : undefined}
aria-valuenow={isDeterminate ? valueNow.toString() : undefined}
id={id}
role="progressbar"
>
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/segmented-control/segmented-control.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ describe("calcite-segmented-control", () => {

async function cycleThroughItemsAndAssertValue(keys: "left-right" | "up-down"): Promise<void> {
const [moveBeforeArrowKey, moveAfterArrowKey] =
keys === "left-right" ? ["ArrowLeft", "ArrowRight"] : ["ArrowUp", "ArrowDown"];
keys === "left-right" ? (["ArrowLeft", "ArrowRight"] as const) : (["ArrowUp", "ArrowDown"] as const);
Copy link
Member Author

Choose a reason for hiding this comment

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

Lumina's typings for await element.press(moveAfterArrowKey); are more strict


await element.press(moveAfterArrowKey);
await page.waitForChanges();
Expand Down
10 changes: 5 additions & 5 deletions packages/calcite-components/src/components/table/table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,11 @@ export class Table implements LocalizedComponent, LoadableComponent, T9nComponen
}

@Listen("calciteInternalTableRowFocusRequest")
calciteInternalTableRowFocusEvent(event: TableRowFocusEvent): void {
const cellPosition = event["detail"].cellPosition;
const rowPos = event["detail"].rowPosition;
const destination = event["detail"].destination;
const lastCell = event["detail"].lastCell;
calciteInternalTableRowFocusEvent(event: CustomEvent<TableRowFocusEvent>): void {
Copy link
Member Author

Choose a reason for hiding this comment

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

looks like a mistake in typing.
TableRowFocusEvent is the payload type, not event type

const cellPosition = event.detail.cellPosition;
const rowPos = event.detail.rowPosition;
const destination = event.detail.destination;
const lastCell = event.detail.lastCell;

const visibleBody = this.bodyRows?.filter((row) => !row.hidden);
const visibleAll = this.allRows?.filter((row) => !row.hidden);
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/text-area/text-area.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ describe("calcite-text-area", () => {
await page.setContent("<calcite-text-area></calcite-text-area>");

const element = await page.find("calcite-text-area");
element.setAttribute("max-length", 5);
element.setAttribute("max-length", "5");
Copy link
Member Author

Choose a reason for hiding this comment

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

Stencil's E2EElement.setAttribute had type (string, any)=>void
In Lumina, it is (string, string)=>void so that it matches the HTMLElement.setAttribute type.
I want to align us closer with DOM typings so that migrating to Vitest browser mode is easier

await page.waitForChanges();

await page.keyboard.press("Tab");
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/time-picker/time-picker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export class TimePicker
/**
* @internal
*/
@Event({ cancelable: false }) calciteInternalTimePickerChange: EventEmitter<string>;
@Event({ cancelable: false }) calciteInternalTimePickerChange: EventEmitter<void>;
Copy link
Member Author

Choose a reason for hiding this comment

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

never actually included a string payload type, and no one was listening for it


//--------------------------------------------------------------------------
//
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/tests/commonTests/t9n.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export async function t9n(componentTestSetup: ComponentTestSetup): Promise<void>
return new Response(new Blob([JSON.stringify(fakeEsMessages, null, 2)], { type: "application/json" }));
}

return orig.call(input, init);
return orig.call(window, input, init);
Copy link
Member Author

Choose a reason for hiding this comment

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

First argument to .call is this.
Not sure why this worked before in Stencil, but this causes issues after migration.

};
},
enMessages,
Expand Down
8 changes: 4 additions & 4 deletions packages/calcite-components/src/utils/dom.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,8 +665,8 @@ describe("dom", () => {

let element: HTMLDivElement;
let dispatchTransitionEvent: TransitionEventDispatcher;
let onStartCallback: jest.Mock<any, any, any>;
let onEndCallback: jest.Mock<any, any, any>;
let onStartCallback: jest.Mock;
Copy link
Member Author

Choose a reason for hiding this comment

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

With this change, the type would be equivalent both in Jest and Vite.
Otherwise it was causing issues because Vite's Mock has only 2 type arguments

let onEndCallback: jest.Mock;

beforeEach(() => {
dispatchTransitionEvent = createTransitionEventDispatcher();
Expand Down Expand Up @@ -766,8 +766,8 @@ describe("dom", () => {

let element: HTMLDivElement;
let dispatchAnimationEvent: AnimationEventDispatcher;
let onStartCallback: jest.Mock<any, any, any>;
let onEndCallback: jest.Mock<any, any, any>;
let onStartCallback: jest.Mock;
let onEndCallback: jest.Mock;

beforeEach(() => {
dispatchAnimationEvent = createAnimationEventDispatcher();
Expand Down
Loading