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

Spike on TS types #992

Closed
wants to merge 4 commits into from
+214 −60
Closed

Spike on TS types #992

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
68ef83c
Run tsc as linter
gitKrystan Nov 29, 2022
2e1cc57
--wip-- [skip ci]
gitKrystan Nov 30, 2022
2c66fa4
wip
gitKrystan Dec 2, 2022
b19b306
WIP
gitKrystan Dec 3, 2022
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
/npm-debug.log*
/testem.log
/yarn-error.log
/.yalc
/yalc.lock

# ember-try
/.node_modules.ember-try/
Expand Down
4 changes: 3 additions & 1 deletion addon-test-support/adapter.js → addon-test-support/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// @ts-nocheck FIXME: Remove

import Ember from 'ember';
import * as QUnit from 'qunit';
import hasEmberVersion from '@ember/test-helpers/has-ember-version';
import { hasEmberVersion } from '@ember/test-helpers';
gitKrystan marked this conversation as resolved.
Show resolved Hide resolved

function unhandledRejectionAssertion(current, error) {
let message, source;
Expand Down
2 changes: 2 additions & 0 deletions addon-test-support/index.js → addon-test-support/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// @ts-nocheck FIXME: Remove

/* globals Testem */

export { default as QUnitAdapter, nonTestDoneCallback } from './adapter';
Expand Down
2 changes: 2 additions & 0 deletions ...test-support/test-isolation-validation.js → ...test-support/test-isolation-validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// @ts-nocheck FIXME: Remove

/* eslint-disable no-console */
import * as QUnit from 'qunit';
import { _cancelTimers as cancelTimers } from '@ember/runloop';
Expand Down
2 changes: 2 additions & 0 deletions addon-test-support/test-loader.js → addon-test-support/test-loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// @ts-nocheck FIXME: Remove

import * as QUnit from 'qunit';
import AbstractTestLoader, {
addModuleExcludeMatcher,
Expand Down
23 changes: 20 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@
"lint:fix": "npm-run-all --print-name --aggregate-output --continue-on-error --parallel \"lint:*:fix\"",
"lint:js": "eslint . --cache",
"lint:js:fix": "eslint . --fix",
"lint:ts": "tsc --noEmit",
"start": "ember serve",
"test": "npm-run-all --print-name \"lint\" \"test:*\"",
"test:ember": "ember test",
"test:ember-compatibility": "ember try:each"
"test:ember-compatibility": "ember try:each",
"prepack": "yarn ember ts:precompile",
"postpack": "yarn ember ts:clean"
},
"dependencies": {
"broccoli-funnel": "^3.0.8",
Expand All @@ -39,6 +42,7 @@
"ember-auto-import": "^2.4.3",
"ember-cli-babel": "^7.26.11",
"ember-cli-test-loader": "^3.0.0",
"ember-cli-typescript": "^5.2.1",
"resolve-package-path": "^4.0.3",
"silent-error": "^1.1.1",
"validate-peer-dependencies": "^2.1.0"
Expand All @@ -47,9 +51,14 @@
"@babel/core": "^7.20.2",
"@babel/eslint-parser": "^7.19.1",
"@ember/optional-features": "^2.0.0",
"@ember/test-helpers": "^2.8.1",
"@ember/test-helpers": "file:.yalc/@ember/test-helpers",
"@embroider/test-setup": "^1.8.3",
"@glimmer/component": "^1.1.2",
"@glimmer/interfaces": "^0.84.2",
"@glimmer/reference": "^0.84.2",
Comment on lines +57 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IDK why I had to install these but I had a lot of typescript errors in node_modules without them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that seems very bad. I will check out the PR and see if I can understand why.

Copy link
Contributor

Choose a reason for hiding this comment

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

For what it's worth, after removing the yalc bit, I don't see this. I haven't yet tried it with a yalc'd @ember/test-helpers, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try again now that everything is further along.

"@tsconfig/ember": "^1.1.0",
"@types/qunit": "^2.19.3",
"@types/rsvp": "^4.0.4",
"ember-angle-bracket-invocation-polyfill": "^3.0.2",
"ember-cli": "~4.8.0",
"ember-cli-dependency-checker": "^3.3.1",
Expand All @@ -59,7 +68,7 @@
"ember-disable-prototype-extensions": "^1.1.3",
"ember-load-initializers": "^2.1.2",
"ember-resolver": "^8.0.3",
"ember-source": "~4.9.1",
"ember-source": "^4.10.0-beta.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

What needs -beta.2? I assume the Owner types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolver preview types from beta.1

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. I just landed a PR this morning, and will do the back port for it on Monday, which will make it available (along with other Owner types needed) on 4.8 LTS.

"ember-source-channel-url": "^3.0.0",
"ember-try": "^2.0.0",
"eslint": "^8.26.0",
Expand All @@ -73,6 +82,7 @@
"qunit": "^2.19.2",
"release-it": "^15.5.0",
"release-it-lerna-changelog": "^5.0.0",
"typescript": "^4.9.3",
"webpack": "^5.74.0"
},
"peerDependencies": {
Expand Down Expand Up @@ -119,5 +129,12 @@
"volta": {
"node": "14.19.1",
"yarn": "1.22.18"
},
"typesVersions": {
"*": {
"*": [
"test-support/*"
gitKrystan marked this conversation as resolved.
Show resolved Hide resolved
]
}
}
}
14 changes: 14 additions & 0 deletions tests/dummy/app/config/environment.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Type declarations for
* import config from 'my-app/config/environment'
*/
declare const config: {
environment: string;
modulePrefix: string;
podModulePrefix: string;
locationType: 'history' | 'hash' | 'none' | 'auto';
rootURL: string;
APP: Record<string, unknown>;
};

export default config;
45 changes: 45 additions & 0 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{
"extends": "@tsconfig/ember/tsconfig.json",
"compilerOptions": {

// The combination of `baseUrl` with `paths` allows Ember's classic package
// layout, which is not resolvable with the Node resolution algorithm, to
// work with TypeScript.
"baseUrl": ".",
"paths": {
"dummy/tests/*": [
"tests/*"
],
"dummy/*": [
"tests/dummy/app/*",
"app/*"
],
"ember-qunit": [
"addon"
],
"ember-qunit/*": [
"addon/*"
],
"ember-qunit/test-support": [
"addon-test-support"
],
"ember-qunit/test-support/*": [
"addon-test-support/*"
],
"*": [
"types/*"
]
}
},
"include": [
"app/**/*",
"addon/**/*",
"tests/**/*",
"types/**/*",
"test-support/**/*",
"addon-test-support/**/*"
],

// FIXME: Remove
"noEmitOnError": false,
}
3 changes: 3 additions & 0 deletions types/dummy/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// FIXME: Not sure this actually does anything
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See https://github.com/emberjs/ember-test-helpers/pull/1272/files#diff-a13bad01b7436c90fb6e3eafacf9947547a2fb96f8abc38588b1f06fcc7e2d55

Copy link
Contributor

Choose a reason for hiding this comment

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

This definitely should do a thing, but it depends on how paths is set I think. This may be basically-invisible as it stands; this is a problem with our blueprint and config that has existed for a very long time.

import 'ember-source/types';
import 'ember-source/types/preview';
7 changes: 7 additions & 0 deletions types/global.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Types for compiled templates
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can probably remove this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep—in fact we definitely want to! They don't get us anything useful in most apps/addons, but they are completely useless in this one in particular I think.

declare module 'ember-qunit/templates/*' {
import { TemplateFactory } from 'ember-cli-htmlbars';

const tmpl: TemplateFactory;
export default tmpl;
}
Loading