Skip to content

Update tsconfig and corresponding docs #1488

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

Merged
merged 6 commits into from
Mar 3, 2022
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
73 changes: 59 additions & 14 deletions blueprint-files/ember-cli-typescript/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,70 @@
{
"compilerOptions": {
"target": "es2020",
"allowJs": true,
"target": "ES2021",
"module": "ES2020",
"moduleResolution": "node",
Copy link
Member

Choose a reason for hiding this comment

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

Not actionable yet, but I suspect we're going to want to change this to node12 as soon as that graduates from nightly, since Webpack 5 (and so by extension, ember-auto-import and Embroider) honor exports config.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. Hoping they ship that in 4.7 and we can flip it on. 🤞🏼

"allowSyntheticDefaultImports": true,
"noImplicitAny": true,
"noImplicitThis": true,
"alwaysStrict": true,
"strictNullChecks": true,
"strictPropertyInitialization": true,
"noFallthroughCasesInSwitch": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,

// Trying to check Ember apps and addons with `allowJs: true` is a recipe
// for many unresolveable type errors, because with *considerable* extra
// configuration it ends up including many files which are *not* valid and
// cannot be: they *appear* to be resolve-able to TS, but are in fact not in
// valid Node-resolveable locations and may not have TS-ready types. This
// will likely improve over time
"allowJs": false,

// --- TS for SemVer Types compatibility
// Strictness settings -- you should *not* change these: Ember code is not
// guaranteed to type check with these set to looser values.
"strict": true,
"noUncheckedIndexedAccess": true,

// Interop: these are viral and will require anyone downstream of your
// package to *also* set them to true. If you *must* enable them to consume
// an upstream package, you should document that for downstream consumers to
// be aware of.
//
// These *are* safe for apps to enable, since they do not *have* downstream
// consumers; but leaving them off is still preferred when possible, since
// it makes it easier to switch between apps and addons and have the same
// rules for what can be imported and how.
"allowSyntheticDefaultImports": false,
"esModuleInterop": false,

// --- Lint-style rules

// TypeScript also supplies some lint-style checks; nearly all of them are
// better handled by ESLint with the `@typescript-eslint`. This one is more
// like a safety check, though, so we leave it on.
"noPropertyAccessFromIndexSignature": true,

// --- Compilation/integration settings
// Setting `noEmitOnError` here allows ember-cli-typescript to catch errors
// and inject them into Ember CLI's build error reporting, which provides
// nice feedback for when
"noEmitOnError": true,

// We use Babel for emitting runtime code, because it's very important that
// we always and only use the same transpiler for non-stable features, in
// particular decorators. If you were to change this to `true`, it could
// lead to accidentally generating code with `tsc` instead of Babel, and
// could thereby result in broken code at runtime.
"noEmit": true,

// Ember makes heavy use of decorators; TS does not support them at all
// without this flag.
"experimentalDecorators": true,

// Support generation of source maps. Note: you must *also* enable source
// maps in your `ember-cli-babel` config and/or `babel.config.js`.
"declaration": true,
"declarationMap": true,
"inlineSourceMap": true,
"inlineSources": true,
Comment on lines +59 to 62
Copy link
Member

Choose a reason for hiding this comment

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

Given that we set noEmit: true here and use Babel for actually emitting code, do these flags have any effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't actually know! These are what I added four years ago. 😬


// The combination of `baseUrl` with `paths` allows Ember's classic package
// layout, which is not resolveable with the Node resolution algorithm, to
// work with TypeScript.
"baseUrl": ".",
"module": "es6",
"experimentalDecorators": true,
"paths": <%= pathsFor(dasherizedPackageName) %>
},
"include": <%= includes %>
Expand Down
30 changes: 17 additions & 13 deletions docs/ts/using-ts-effectively.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,28 @@ Some specific tips for success on the technical front:

First, use the _strictest_ strictness settings that our typings allow (currently all strictness settings except `strictFunctionTypes`). While it may be tempting to start with the _loosest_ strictness settings and then to tighten them down as you go, this will actually mean that "getting your app type-checking" will become a repeated process—getting it type-checking with every new strictness setting you enable—rather than something you do just once.

The full recommended _strictness_ settings in your `"compilerOptions"` hash:
The full recommended _strictness_ settings in your `"compilerOptions"` hash (which are also the settings generated by the ember-cli-typescript blueprint):

```json
```json5
{
"noImplicitAny": true,
"noImplicitThis": true,
"alwaysStrict": true,
"strictNullChecks": true,
"strictPropertyInitialization": true,
"noFallthroughCasesInSwitch": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noUncheckedIndexedAccess": true,
"compilerOptions": {
// Strictness settings -- you should *not* change these: Ember code is not
// guaranteed to type check with these set to looser values.
"strict": true,
"noUncheckedIndexedAccess": true,

// You should feel free to change these, especially if you are already
// covering them via linting (e.g. with @typescript-eslint).
"noFallthroughCasesInSwitch": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noPropertyAccessFromIndexSignature": true,
}
}
```

A good approach is to start at your "leaf" files (the ones that don't import anything else from your app, only Ember types) and then work your way back inward toward the most core types that are used everywhere. Often the highest-value modules are your Ember Data models and any core services that are used everywhere else in the app – and those are also the ones that tend to have the most cascading effects (having to update _tons_ of other places in your app) when you type them later in the process.
A good approach is to start at your "leaf" modules (the ones that don't import anything else from your app, only Ember or third-party types) and then work your way back inward toward the most core modules that are used everywhere. Often the highest-value modules are your Ember Data models and any core services that are used everywhere else in the app – and those are also the ones that tend to have the most cascading effects (having to update _tons_ of other places in your app) when you type them later in the process.

Finally, leave `"noEmitOnError": true` (the default) in the `"compilerOptions"` hash in your `tsconfig.json`. This will fail your build if you have type errors, which gives you the fastest feedback as you add types.

Expand Down
11 changes: 6 additions & 5 deletions ts/tests/blueprints/ember-cli-typescript-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import path from 'path';
import helpers from 'ember-cli-blueprint-test-helpers/helpers';
import chaiHelpers from 'ember-cli-blueprint-test-helpers/chai';
import Blueprint from 'ember-cli/lib/models/blueprint';
import ts from 'typescript';

import { setupPublishedVersionStashing } from '../helpers/stash-published-version';
import ects from '../../blueprints/ember-cli-typescript/index';
Expand Down Expand Up @@ -71,7 +72,7 @@ describe('Acceptance: ember-cli-typescript generator', function () {
const tsconfig = file('tsconfig.json');
expect(tsconfig).to.exist;

const tsconfigJson = JSON.parse(tsconfig.content);
const tsconfigJson = ts.parseConfigFileTextToJson('tsconfig.json', tsconfig.content).config;
expect(tsconfigJson.compilerOptions.paths).to.deep.equal({
'my-app/tests/*': ['tests/*'],
'my-app/*': ['app/*'],
Expand Down Expand Up @@ -121,7 +122,7 @@ describe('Acceptance: ember-cli-typescript generator', function () {
const tsconfig = file('tsconfig.json');
expect(tsconfig).to.exist;

const tsconfigJson = JSON.parse(tsconfig.content);
const tsconfigJson = ts.parseConfigFileTextToJson('tsconfig.json', tsconfig.content).config;
expect(tsconfigJson.compilerOptions.paths).to.deep.equal({
'dummy/tests/*': ['tests/*'],
'dummy/*': ['tests/dummy/app/*', 'app/*'],
Expand Down Expand Up @@ -197,7 +198,7 @@ describe('Acceptance: ember-cli-typescript generator', function () {
const tsconfig = file('tsconfig.json');
expect(tsconfig).to.exist;

const json = JSON.parse(tsconfig.content);
const json = ts.parseConfigFileTextToJson('tsconfig.json', tsconfig.content).config;
expect(json.compilerOptions.paths).to.deep.equal({
'my-app/tests/*': ['tests/*'],
'my-app/*': ['app/*', 'lib/my-addon-1/app/*', 'lib/my-addon-2/app/*'],
Expand Down Expand Up @@ -243,7 +244,7 @@ describe('Acceptance: ember-cli-typescript generator', function () {
const tsconfig = file('tsconfig.json');
expect(tsconfig).to.exist;

const json = JSON.parse(tsconfig.content);
const json = ts.parseConfigFileTextToJson('tsconfig.json', tsconfig.content).config;
expect(json.compilerOptions.paths).to.deep.equal({
'my-app/tests/*': ['tests/*'],
'my-app/mirage/*': ['mirage/*'],
Expand All @@ -269,7 +270,7 @@ describe('Acceptance: ember-cli-typescript generator', function () {
const tsconfig = file('tsconfig.json');
expect(tsconfig).to.exist;

const json = JSON.parse(tsconfig.content);
const json = ts.parseConfigFileTextToJson('tsconfig.json', tsconfig.content).config;
expect(json.compilerOptions.paths).to.deep.equal({
'dummy/tests/*': ['tests/*'],
'dummy/mirage/*': ['tests/dummy/mirage/*'],
Expand Down
29 changes: 21 additions & 8 deletions ts/tests/commands/precompile-test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import * as fs from 'fs-extra';
import * as path from 'path';
import { EOL } from 'os';
import { hook } from 'capture-console';
import ember from 'ember-cli-blueprint-test-helpers/lib/helpers/ember';
import blueprintHelpers from 'ember-cli-blueprint-test-helpers/helpers';
import ts from 'typescript';

const setupTestHooks = blueprintHelpers.setupTestHooks;
const emberNew = blueprintHelpers.emberNew;

Expand All @@ -26,7 +29,9 @@ describe('Acceptance: ts:precompile command', function () {

let declaration = file('test-file.d.ts');
expect(declaration).to.exist;
expect(declaration.content.trim()).to.equal(`export declare const testString: string;`);
expect(declaration.content.trim()).to.equal(
`export declare const testString: string;${EOL}//# sourceMappingURL=test-file.d.ts.map`
);
});

it('generates nothing from the app tree', async () => {
Expand Down Expand Up @@ -63,7 +68,7 @@ describe('Acceptance: ts:precompile command', function () {
fs.writeFileSync('src/test-file.ts', `export const testString: string = 'hello';`);

let pkg = fs.readJsonSync('package.json');
let tsconfig = fs.readJSONSync('tsconfig.json');
let tsconfig = ts.readConfigFile('tsconfig.json', ts.sys.readFile).config;
tsconfig.include.push('src');
tsconfig.compilerOptions.paths[`${pkg.name}/src/*`] = ['src/*'];
fs.writeJSONSync('tsconfig.json', tsconfig);
Expand All @@ -72,7 +77,9 @@ describe('Acceptance: ts:precompile command', function () {

let declaration = file('src/test-file.d.ts');
expect(declaration).to.exist;
expect(declaration.content.trim()).to.equal(`export declare const testString: string;`);
expect(declaration.content.trim()).to.equal(
`export declare const testString: string;${EOL}//# sourceMappingURL=test-file.d.ts.map`
);
});
});

Expand All @@ -85,7 +92,7 @@ describe('Acceptance: ts:precompile command', function () {
);

let pkg = fs.readJsonSync('package.json');
let tsconfig = fs.readJSONSync('tsconfig.json');
let tsconfig = ts.readConfigFile('tsconfig.json', ts.sys.readFile).config;
tsconfig.include.push('src');
tsconfig.compilerOptions.paths[`${pkg.name}/*`] = ['addon-test-support/*'];
fs.writeJSONSync('tsconfig.json', tsconfig);
Expand All @@ -94,7 +101,9 @@ describe('Acceptance: ts:precompile command', function () {

let declaration = file('test-file.d.ts');
expect(declaration).to.exist;
expect(declaration.content.trim()).to.equal(`export declare const testString: string;`);
expect(declaration.content.trim()).to.equal(
`export declare const testString: string;${EOL}//# sourceMappingURL=test-file.d.ts.map`
);
});
});

Expand All @@ -120,7 +129,9 @@ describe('Acceptance: ts:precompile command', function () {
return ember(['ts:precompile']).then(() => {
const declaration = file('test-support/test-file.d.ts');
expect(declaration).to.exist;
expect(declaration.content.trim()).to.equal(`export declare const testString: string;`);
expect(declaration.content.trim()).to.equal(
`export declare const testString: string;${EOL}//# sourceMappingURL=test-file.d.ts.map`
);
});
});

Expand Down Expand Up @@ -154,12 +165,14 @@ describe('Acceptance: ts:precompile command', function () {
return ember(['ts:precompile']).then(() => {
const componentDecl = file('components/my-component.d.ts');
expect(componentDecl).to.exist;
expect(componentDecl.content.trim()).to.equal(`export declare const testString: string;`);
expect(componentDecl.content.trim()).to.equal(
`export declare const testString: string;${EOL}//# sourceMappingURL=my-component.d.ts.map`
);

const testSupportDecl = file('test-support/test-file.d.ts');
expect(testSupportDecl).to.exist;
expect(testSupportDecl.content.trim()).to.equal(
`export declare const anotherTestString: string;`
`export declare const anotherTestString: string;${EOL}//# sourceMappingURL=test-file.d.ts.map`
);
});
});
Expand Down
26 changes: 24 additions & 2 deletions ts/tests/helpers/stash-published-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@ import fs from 'fs-extra';

const PACKAGE_PATH = 'node_modules/ember-cli-typescript';

function isNodeError(e: unknown): e is NodeJS.ErrnoException {
// Not full-proof but good enough for our purposes.
return e instanceof Error && 'code' in e;
}

function handleError(e: unknown) {
// Ignore the case where we just don't have the files to move. (And hope this
// works correctly?)
if (!isNodeError(e) || e.code !== 'ENOENT') {
throw e;
}
}

/**
* We have assorted devDependencies that themselves depend on `ember-cli-typescript`.
* This means we have a published copy of the addon present in `node_modules` normally,
Expand All @@ -12,10 +25,19 @@ const PACKAGE_PATH = 'node_modules/ember-cli-typescript';
*/
export function setupPublishedVersionStashing(hooks: Mocha.Suite): void {
hooks.beforeAll(async () => {
await fs.move(PACKAGE_PATH, `${PACKAGE_PATH}.published`);
fs.move(PACKAGE_PATH, `${PACKAGE_PATH}.published`).catch();
try {
await fs.move(PACKAGE_PATH, `${PACKAGE_PATH}.published`);
} catch (e: unknown) {
handleError(e);
}
});

hooks.afterAll(async () => {
await fs.move(`${PACKAGE_PATH}.published`, PACKAGE_PATH);
try {
await fs.move(`${PACKAGE_PATH}.published`, PACKAGE_PATH);
} catch (e) {
handleError(e);
}
});
}