Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 14 additions & 14 deletions packages/qunit-dom/lib/assertions.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import exists from './assertions/exists';
import focused from './assertions/focused';
import notFocused from './assertions/not-focused';
import isChecked from './assertions/is-checked';
import isNotChecked from './assertions/is-not-checked';
import isRequired from './assertions/is-required';
import isNotRequired from './assertions/is-not-required';
import isValid from './assertions/is-valid';
import isVisible from './assertions/is-visible';
import isDisabled from './assertions/is-disabled';
import matchesSelector from './assertions/matches-selector';
import elementToString from './helpers/element-to-string';
import collapseWhitespace from './helpers/collapse-whitespace';
import { toArray } from './helpers/node-list';
import exists from './assertions/exists.js';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

extensions are required by the "node16" moduleResolution option.
(some browser projects (and whole ecosystems) also use this)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about moduleSuffixes to make it automatic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the need here isn't for TS, exactly, I should have provided more context. When a package is using node16-style resolution (not necessarily the TS config option), all imports must have extensions. Bundlers generally abstract this away.

import focused from './assertions/focused.js';
import notFocused from './assertions/not-focused.js';
import isChecked from './assertions/is-checked.js';
import isNotChecked from './assertions/is-not-checked.js';
import isRequired from './assertions/is-required.js';
import isNotRequired from './assertions/is-not-required.js';
import isValid from './assertions/is-valid.js';
import isVisible from './assertions/is-visible.js';
import isDisabled from './assertions/is-disabled.js';
import matchesSelector from './assertions/matches-selector.js';
import elementToString from './helpers/element-to-string.js';
import collapseWhitespace from './helpers/collapse-whitespace.js';
import { toArray } from './helpers/node-list.js';

export interface AssertionResult {
result: boolean;
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/exists.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ExistsOptions } from '../assertions';
import type { ExistsOptions } from '../assertions.js';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the need for type here is enforced from verbatimModuleSyntax -- more information in the tsconfig.json


export default function exists(options?: ExistsOptions | string, message?: string) {
let expectedCount: number | null = null;
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/focused.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import elementToString from '../helpers/element-to-string';
import elementToString from '../helpers/element-to-string.js';

export default function focused(message?: string) {
let element = this.findTargetElement();
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/is-checked.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import elementToString from '../helpers/element-to-string';
import elementToString from '../helpers/element-to-string.js';

export default function checked(message?: string) {
let element = this.findTargetElement();
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/is-not-checked.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import elementToString from '../helpers/element-to-string';
import elementToString from '../helpers/element-to-string.js';

export default function notChecked(message?: string) {
let element = this.findTargetElement();
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/is-not-required.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import elementToString from '../helpers/element-to-string';
import elementToString from '../helpers/element-to-string.js';

export default function notRequired(message?: string) {
let element = this.findTargetElement();
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/is-required.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import elementToString from '../helpers/element-to-string';
import elementToString from '../helpers/element-to-string.js';

export default function required(message?: string) {
let element = this.findTargetElement();
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/assertions/is-valid.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import elementToString from '../helpers/element-to-string';
import elementToString from '../helpers/element-to-string.js';

export default function isValid(message?: string, options: { inverted?: boolean } = {}) {
let element = this.findTargetElement();
Expand Down
4 changes: 2 additions & 2 deletions packages/qunit-dom/lib/assertions/is-visible.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import visible from '../helpers/visible';
import { ExistsOptions } from '../assertions';
import visible from '../helpers/visible.js';
import type { ExistsOptions } from '../assertions.js';

export default function isVisible(options?: string | ExistsOptions, message?: string) {
let expectedCount: number | null = null;
Expand Down
2 changes: 1 addition & 1 deletion packages/qunit-dom/lib/helpers/test-assertions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import DOMAssertions, { AssertionResult } from '../assertions';
import DOMAssertions, { type AssertionResult } from '../assertions.js';

export default class TestAssertions {
public results: AssertionResult[] = [];
Expand Down
4 changes: 2 additions & 2 deletions packages/qunit-dom/lib/install.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import DOMAssertions from './assertions';
import { getRootElement } from './root-element';
import DOMAssertions from './assertions.js';
import { getRootElement } from './root-element.js';

declare global {
interface Assert {
Expand Down
6 changes: 3 additions & 3 deletions packages/qunit-dom/lib/qunit-dom-modules.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import install from './install';
import { overrideRootElement } from './root-element';
import install from './install.js';
import { overrideRootElement } from './root-element.js';

export { default as install } from './install';
export { default as install } from './install.js';

interface SetupOptions {
getRootElement?: () => Element | null;
Expand Down
4 changes: 2 additions & 2 deletions packages/qunit-dom/lib/qunit-dom.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global QUnit */
import install from './install';
import install from './install.js';

export { setup } from './qunit-dom-modules';
export { setup } from './qunit-dom-modules.js';

install(QUnit.assert);
7 changes: 1 addition & 6 deletions packages/qunit-dom/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@
"default": "./dist/es/index.js"
}
},
"typesVersions": {
Copy link
Copy Markdown
Contributor Author

@NullVoxPopuli NullVoxPopuli Oct 9, 2023

Choose a reason for hiding this comment

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

turns out this wasn't needed -- the types field is a sufficient fallback for environments that don't support exports
(moduleResolution: node)

"*": {
"*": "dist/es/qunit-dom.d.ts"
}
},
"types": "dist/es/qunit-dom.d.ts",
"files": [
"dist"
Expand All @@ -35,7 +30,7 @@
"lint": "concurrently 'npm:lint:*(!fix)' --names 'lint:'",
"lint:fix": "concurrently 'npm:lint:*:fix' --names 'fix:'",
"lint:package": "publint",
"lint:published-types": "attw --pack --ignore-rules no-resolution cjs-resolves-to-esm internal-resolution-error",
"lint:published-types": "attw --pack --ignore-rules cjs-resolves-to-esm internal-resolution-error",
"lint:js": "eslint . --cache",
"lint::js:fix": "eslint . --fix",
"prepublish": "rollup -c",
Expand Down
10 changes: 10 additions & 0 deletions packages/qunit-dom/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@
"compilerOptions": {
"noImplicitAny": true,
"declaration": true,
// Forces import type when imports are only used as types.
"verbatimModuleSyntax": true,
// We build to ESM, so we need to tell TS that we build to ESM
// otherwise TS uses fake ESM (you use imports, but it assumes the compilation is to CJS)
// These are also required when using
// verbatimModuleSyntax, because it's an
// ESM-only feature
"target": "esnext",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NullVoxPopuli on line 19 there's "target": "es5" so this one is overridden

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks! #2080

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tbf it does stand out 😅

❯ pnpm test

> qunit-dom@3.0.0 test (…)/qunit-dom/packages/qunit-dom
> vitest

▲ [WARNING] Duplicate key "target" in object literal [duplicate-object-key]

    tsconfig.json:19:4:
      19 │     "target": "es5"
         ╵     ~~~~~~~~

  The original key "target" is here:

    tsconfig.json:12:4:
      12 │     "target": "esnext",
         ╵     ~~~~~~~~

"module": "esnext",

"lib": [
"ES6",
"DOM"
Expand Down
22 changes: 22 additions & 0 deletions packages/test-types-resolution-node/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "type-tests-resolution-node",
"private": true,
"type": "module",
"scripts": {
"lint": "tsc --noEmit",
"start": "tsc --noEmit --watch",
"test": "echo 'run pnpm lint' instead"
},
"devDependencies": {
"@types/qunit": "2.19.6",
"expect-type": "^0.17.3",
"typescript": "5.2.2"
},
"volta": {
"extends": "../../package.json"
},
"dependencies": {
"qunit": "^2.20.0",
"qunit-dom": "workspace:*"
}
}
10 changes: 10 additions & 0 deletions packages/test-types-resolution-node/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import QUnit from 'qunit';
import { setup } from 'qunit-dom';
import { expectTypeOf } from 'expect-type'

setup(QUnit.assert);

expectTypeOf(QUnit.assert.dom).parameter(0).toEqualTypeOf<string | Element | null | undefined>();
// @ts-expect-error - there is only one parameter
expectTypeOf(QUnit.assert.dom).parameter(1).toEqualTypeOf<never>();

25 changes: 25 additions & 0 deletions packages/test-types-resolution-node/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"compilerOptions": {
// Settings needed for test
"target": "esnext",
// using qunit-dom with node would require jsdom or happydom
// but these use cases are currently untested.
// But some older packagers set up TS this way because they
// haven't moved to the newer "bundler" resolution mode yet.
"module": "esnext",
"moduleResolution": "node",
"allowSyntheticDefaultImports": true,

/////////////////////////
// Settings to help us type check effectively.
// Unrelated to the above.

// Strictness settings (one day default?)
"strict": true,

// Don't implicitly pull in declarations from `@types` packages unless we
// actually import from them AND the package in question doesn't bring its
// own types.
"types": []
}
}
22 changes: 22 additions & 0 deletions packages/test-types-resolution-node16/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "type-tests-resolution-node16",
"private": true,
"type": "module",
"scripts": {
"lint": "tsc --noEmit",
"start": "tsc --noEmit --watch",
"test": "echo 'run pnpm lint' instead"
},
"devDependencies": {
"@types/qunit": "2.19.6",
"expect-type": "^0.17.3",
"typescript": "5.2.2"
},
"volta": {
"extends": "../../package.json"
},
"dependencies": {
"qunit": "^2.20.0",
"qunit-dom": "workspace:*"
}
}
10 changes: 10 additions & 0 deletions packages/test-types-resolution-node16/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import QUnit from 'qunit';
import { setup } from 'qunit-dom';
import { expectTypeOf } from 'expect-type'

setup(QUnit.assert);

expectTypeOf(QUnit.assert.dom).parameter(0).toEqualTypeOf<string | Element | null | undefined>();
// @ts-expect-error - there is only one parameter
expectTypeOf(QUnit.assert.dom).parameter(1).toEqualTypeOf<never>();

23 changes: 23 additions & 0 deletions packages/test-types-resolution-node16/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"compilerOptions": {
// Settings needed for test
"target": "esnext",
// using qunit-dom with node would require jsdom or happydom
// but these use cases are currently untested.
// Node16+ requires file extensions
"module": "Node16",
"moduleResolution": "node16",

/////////////////////////
// Settings to help us type check effectively.
// Unrelated to the above.

// Strictness settings (one day default?)
"strict": true,

// Don't implicitly pull in declarations from `@types` packages unless we
// actually import from them AND the package in question doesn't bring its
// own types.
"types": []
}
}
22 changes: 22 additions & 0 deletions packages/test-types/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "type-tests",
"private": true,
"type": "module",
"scripts": {
"lint": "tsc --noEmit",
"start": "tsc --noEmit --watch",
"test": "echo 'run pnpm lint' instead"
},
"devDependencies": {
"@types/qunit": "2.19.6",
"expect-type": "^0.17.3",
"typescript": "5.2.2"
},
"volta": {
"extends": "../../package.json"
},
"dependencies": {
"qunit": "^2.20.0",
"qunit-dom": "workspace:*"
}
}
10 changes: 10 additions & 0 deletions packages/test-types/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import QUnit from 'qunit';
import { setup } from 'qunit-dom';
import { expectTypeOf } from 'expect-type'

setup(QUnit.assert);

expectTypeOf(QUnit.assert.dom).parameter(0).toEqualTypeOf<string | Element | null | undefined>();
// @ts-expect-error - there is only one parameter
expectTypeOf(QUnit.assert.dom).parameter(1).toEqualTypeOf<never>();

20 changes: 20 additions & 0 deletions packages/test-types/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"compilerOptions": {
// Settings needed for test
"target": "esnext",
"module": "esnext",
"moduleResolution": "bundler",

/////////////////////////
// Settings to help us type check effectively.
// Unrelated to the above.

// Strictness settings (one day default?)
"strict": true,

// Don't implicitly pull in declarations from `@types` packages unless we
// actually import from them AND the package in question doesn't bring its
// own types.
"types": []
}
}
Loading