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

Introduced TypeScript (Part 4) #120

Merged
merged 12 commits into from
Jun 3, 2022
Merged

Introduced TypeScript (Part 4) #120

merged 12 commits into from
Jun 3, 2022

Conversation

ijlee2
Copy link
Owner

@ijlee2 ijlee2 commented Jun 1, 2022

Description

The demo app, which is heavily used for testing ember-container-query, is a fairly complex Ember application. To help onboard contributors, I'd like to introduce TypeScript to better document code.

Rather than converting the entire demo app to TypeScript at once (which would result in a large pull request that's hard to understand when we go back to), I'll split the task into a few.

In Part 4, JavaScript files in the following directories have been converted to TypeScript:

  • tests/acceptance
  • tests/helpers
  • tests/integration
  • tests/unit

Notes

Over 80%!

References

@ijlee2 ijlee2 added the enhance: documentation Issue asks for better documentation (e.g. README, code, tests) label Jun 1, 2022
@ijlee2 ijlee2 force-pushed the add-typescript-support-part-4 branch 4 times, most recently from 1cf7aa9 to d551e6f Compare June 3, 2022 12:07
@ijlee2 ijlee2 force-pushed the add-typescript-support-part-4 branch from c18fba2 to 1266a9d Compare June 3, 2022 13:39
@ijlee2 ijlee2 marked this pull request as ready for review June 3, 2022 13:42
Comment on lines +11 to +18
export interface CustomAssert extends Assert {
areDataAttributesCorrect?: (dataAttributes: DataAttributes) => void;
areDimensionsCorrect?: (
expectedWidth: number,
expectedHeight: number
) => void;
areFeaturesCorrect?: (features: Features) => void;
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

I couldn't find online an example of how to type custom assertions. By trial and error, I came up with this interface. The ?: was needed for the delete operator not to cause a TypeScript error.

Comment on lines +6 to +10
import EmberResolver from 'ember-resolver';

interface SetupTestOptions {
resolver?: EmberResolver | undefined;
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

export default async function takeSnapshot(qunitAssert, options = {}) {
checkInput(qunitAssert, options);
export default async function takeSnapshot(
qunitAssert: unknown,
Copy link
Owner Author

Choose a reason for hiding this comment

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

Somehow, the assert object, which originally should be of type Assert (from qunit, possibly extended by qunit-dom), is transformed into an object whose type definition I couldn't find:

I thought it'd be best to specify the type as unknown, then cast it into something (called QUnitAssert here) before we pass it to getName().

Comment on lines -77 to -81
assert(
"`qunitAssert` must be QUnit's assert object.",
typeof qunitAssert === 'object' && !!qunitAssert.test
);

Copy link
Owner Author

Choose a reason for hiding this comment

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

I deleted these lines because I shouldn't have tested a third-party's internal implementation.

Comment on lines -16 to -21
this.fetchData = () => {
assert.ok(
true,
'{{did-insert}} modifier works. (But we should find a better way to separate concerns!)'
);
};
Copy link
Owner Author

Choose a reason for hiding this comment

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

This removes the test being dependent on @ember/render-modifiers to have been installed.

@ijlee2 ijlee2 merged commit 2a18b44 into main Jun 3, 2022
@ijlee2 ijlee2 deleted the add-typescript-support-part-4 branch June 3, 2022 14:01
@@ -5,6 +5,7 @@ import { setRunOptions } from 'ember-a11y-testing/test-support';
import { start } from 'ember-qunit';
import * as QUnit from 'qunit';
import { setup } from 'qunit-dom';
import 'qunit-dom';
Copy link
Owner Author

Choose a reason for hiding this comment

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

qunit-dom suggested (at the time of writing) adding this import statement so that assert takes the correct type (i.e. assert.dom becomes well-defined).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhance: documentation Issue asks for better documentation (e.g. README, code, tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant