Add support for @tsconfig/ember 3.0.0 #614
Conversation
mayatron
commented
Jul 26, 2023
mayatron
commented
- Add support for @tsconfig/ember 3.0.0
- Addresses Fix support for @tsconfig/ember 3.0.0 error TS7016 #613
- Add `@tsconfig/[email protected]` to `test-app` - Fix minor TypeScript issues in tests
- Addresses #613 - Update `ember-try` with new scenario to cover latest TypeScript 5 and `@tsconfig/ember` configuration
| "compilerOptions": { | ||
| "target": "es2020", | ||
| "allowJs": true, | ||
| "moduleResolution": "node", |
There was a problem hiding this comment.
I'd like to make sure we still support the moduleResolution: "node". Also, I think leaving this test app in a "legacy" state makes sense until we decide to increase the bottom line of the support matrix.
Maybe a better way to test would be just to create a new cutting-edge test app, with just a single dummy test that imports some ember-cli-page-object module + tsc as a test.
...Or, alternatively, we could create a "legacy" test app with that dummy test + tsc, but then I think it makes sense to upgrade the current test app to the latest, which seems to be just more work to do 🤔
Thoughts?
| @@ -94,7 +94,7 @@ module('create', function () { | |||
| // @ts-expect-error violate types to check if it fails in weakly-typed envs | |||
| create(''); | |||
| assert.true(false, 'should error'); | |||
| } catch (e) { | |||
| } catch (e: any) { | |||
There was a problem hiding this comment.
can we have Error instead of any?
There was a problem hiding this comment.
We can't use the type Error here, due to:
Catch clause variable type annotation must be 'any' or 'unknown' if specified.ts(1196)
This is an annoying issue with TypeScript, see a long discussion thread here. Alternatively, and probably more correct, we can type this as unknown.
| @@ -36,6 +36,7 @@ | |||
| "@embroider/test-setup": "^1.8.0", | |||
| "@glimmer/component": "^1.1.2", | |||
| "@glimmer/tracking": "^1.1.2", | |||
| "@tsconfig/ember": "^2.0.0", | |||
There was a problem hiding this comment.
looks like we should remove this from the package.json or from the ember-try scenario. I tend to remove it from the package.json in scope of the current test-app
|
@ro0gr let me know if you have any questions or feedback about my comments. Happy to discuss and iterate with you on this! |
|
@mayatron I do like your change. I'm going to merge and release it tomorrow. |
|
Just published it as 2.1.0. Thanks for your effort! |
