-
Notifications
You must be signed in to change notification settings - Fork 125
v3: Maximize compatibility #2065
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
Changes from 1 commit
35a42a4
ded8b21
4fa3e1a
5eb98b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { ExistsOptions } from '../assertions'; | ||
| import type { ExistsOptions } from '../assertions.js'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the need for type here is enforced from |
||
|
|
||
| export default function exists(options?: ExistsOptions | string, message?: string) { | ||
| let expectedCount: number | null = null; | ||
|
|
||
| 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); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,11 +20,6 @@ | |
| "default": "./dist/es/index.js" | ||
| } | ||
| }, | ||
| "typesVersions": { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| "*": { | ||
| "*": "dist/es/qunit-dom.d.ts" | ||
| } | ||
| }, | ||
| "types": "dist/es/qunit-dom.d.ts", | ||
| "files": [ | ||
| "dist" | ||
|
|
@@ -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", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NullVoxPopuli on line 19 there's
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks! #2080
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tbf it does stand out 😅 |
||
| "module": "esnext", | ||
|
|
||
| "lib": [ | ||
| "ES6", | ||
| "DOM" | ||
|
|
||
| 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:*" | ||
| } | ||
| } |
| 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>(); | ||
|
|
| 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": [] | ||
| } | ||
| } |
| 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:*" | ||
| } | ||
| } |
| 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>(); | ||
|
|
| 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": [] | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.