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.

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

Enforce @ts-expect-error via eslint #19198

Merged
merged 11 commits into from
Sep 21, 2022
1 change: 0 additions & 1 deletion code/addons/links/src/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ describe('preview', () => {

it('should handle functions returning strings', () => {
const handler = linkTo(
// @ts-expect-error
(a, b) => a + b,
(a, b) => b + a
);
Expand Down
5 changes: 4 additions & 1 deletion code/addons/links/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ const valueOrCall = (args: string[]) => (value: string | ((...args: string[]) =>
typeof value === 'function' ? value(...args) : value;

export const linkTo =
(idOrTitle: string, nameInput?: string | ((...args: any[]) => string)) =>
(
idOrTitle: string | ((...args: any[]) => string),
nameInput?: string | ((...args: any[]) => string)
) =>
(...args: any[]) => {
const resolver = valueOrCall(args);
const title = resolver(idOrTitle);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
import { Component, EventEmitter, Input, Output } from '@angular/core';

export const exportedConstant = 'An exported constant';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-nocheck
Comment on lines +1 to 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like ts-nocheck is actually valid here because of the decorators, so I've allowed it.

/* eslint-disable no-console */
/* eslint-disable no-underscore-dangle */
Expand Down
1 change: 0 additions & 1 deletion code/lib/api/src/modules/versions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import global from 'global';
// @ts-ignore (FIXME should be expect-error no typedefs but fails build --prep)
import semver from '@storybook/semver';
import memoize from 'memoizerific';

Expand Down
1 change: 1 addition & 0 deletions code/lib/api/src/typings.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
declare module 'global';
declare module 'preval.macro';
declare module '@storybook/semver';
IanVS marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions code/lib/builder-webpack5/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"webpack-virtual-modules": "^0.4.3"
},
"devDependencies": {
"@types/case-sensitive-paths-webpack-plugin": "^2.1.6",
"@types/terser-webpack-plugin": "^5.2.0",
"@types/webpack-dev-middleware": "^5.3.0",
"@types/webpack-hot-middleware": "^2.25.6",
Expand Down
9 changes: 4 additions & 5 deletions code/lib/builder-webpack5/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { checkWebpackVersion } from '@storybook/core-webpack';

export * from './types';

let compilation: ReturnType<typeof webpackDevMiddleware>;
let compilation: ReturnType<typeof webpackDevMiddleware> | undefined;
let reject: (reason?: any) => void;

type WebpackBuilder = Builder<Configuration, Stats>;
Expand Down Expand Up @@ -72,7 +72,6 @@ export const bail: WebpackBuilder['bail'] = async () => {
}
// we wait for the compiler to finish it's work, so it's command-line output doesn't interfere
return new Promise((res, rej) => {
// @ts-ignore (FIXME: should be expect-error but fails build --prep)
if (process && compilation) {
try {
compilation.close(() => res());
Expand Down Expand Up @@ -134,7 +133,7 @@ const starter: StarterFunction = async function* starterGeneratorFn({
router.use(webpackHotMiddleware(compiler as any));

const stats = await new Promise<Stats>((ready, stop) => {
compilation.waitUntilValid(ready as any);
compilation?.waitUntilValid(ready as any);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This changes the behavior a little since the original would throw if compilation was undefined while this fails silently. But since compilation is being defined above I think that's okay.

reject = stop;
});
yield;
Expand Down Expand Up @@ -219,10 +218,10 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime,

logger.trace({ message: '=> Preview built', time: process.hrtime(startTime) });
if (stats && stats.hasWarnings()) {
// @ts-ignore (FIXME should be @ts-expect-error but fails build --prep)
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- we know it has warnings because of hasWarnings()
stats
.toJson({ warnings: true } as StatsOptions)
.warnings.forEach((e) => logger.warn(e.message));
.warnings!.forEach((e) => logger.warn(e.message));
}

// https://webpack.js.org/api/node/#run
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import path from 'path';
import { DefinePlugin, HotModuleReplacementPlugin, ProgressPlugin, ProvidePlugin } from 'webpack';
import type { Configuration } from 'webpack';
import HtmlWebpackPlugin from 'html-webpack-plugin';
// @ts-ignore
import CaseSensitivePathsPlugin from 'case-sensitive-paths-webpack-plugin';
import TerserWebpackPlugin from 'terser-webpack-plugin';
import VirtualModulePlugin from 'webpack-virtual-modules';
Expand Down
3 changes: 2 additions & 1 deletion code/lib/builder-webpack5/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"strict": true
"strict": true,
"skipLibCheck": true
Copy link
Contributor Author

@JReinhold JReinhold Sep 15, 2022

Choose a reason for hiding this comment

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

case-sensitive-paths-webpack-plugin has transitive types issues with webpack5 which we want to ignore because we can't do anything about it. Reading the issues in the repo makes it seem like there might actually be some issues with it and webpack5.

},
"include": ["src/**/*", "typings.d.ts"],
"exclude": ["src/**.test.ts"]
Expand Down
1 change: 1 addition & 0 deletions code/lib/core-common/src/typings.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
declare module 'pnp-webpack-plugin';
declare module '@storybook/semver';
declare module 'lazy-universal-dotenv';
1 change: 0 additions & 1 deletion code/lib/core-common/src/utils/envs.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-ignore (FIXME should be "@ts-expect-error does not have defs, but universal-dotenv is in TS now" but fails build --prep)
import { getEnvironment } from 'lazy-universal-dotenv';
import { nodePathsToArray } from './paths';

Expand Down
1 change: 1 addition & 0 deletions code/lib/core-common/src/utils/normalize-stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const DEFAULT_FILES = '**/*.@(mdx|stories.mdx|stories.tsx|stories.ts|stories.jsx
// TODO: remove - LEGACY support for bad glob patterns we had in SB 5 - remove in SB7
const fixBadGlob = deprecate(
(match: RegExpMatchArray) => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore (FIXME should be "@ts-expect-error this will get removed later anyway" but fails build --prep)
return match.input.replace(match[1], `@${match[1]}`);
},
Expand Down
12 changes: 4 additions & 8 deletions code/lib/core-webpack/src/merge-webpack-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ const config: Configuration = {
module: {
rules: [{ use: 'r1' }, { use: 'r2' }],
},
// For snapshot readability purposes `plugins` attribute doesn't match the correct type
// @ts-expect-errors
// @ts-expect-errors For snapshot readability purposes `plugins` attribute doesn't match the correct type
plugins: ['p1', 'p2'],
resolve: {
enforceExtension: true,
Expand Down Expand Up @@ -45,8 +44,7 @@ describe('mergeConfigs', () => {
noParse: /jquery|lodash/,
rules: [{ use: 'r3' }, { use: 'r4' }],
},
// For snapshot readability purposes `plugins` attribute doesn't match the correct type
// @ts-expect-errors
// @ts-expect-errors For snapshot readability purposes `plugins` attribute doesn't match the correct type
plugins: ['p3', 'p4'],
resolve: {
enforceExtension: false,
Expand All @@ -57,8 +55,7 @@ describe('mergeConfigs', () => {
},
},
optimization: {
// For snapshot readability purposes `minimizer` attribute doesn't match the correct type
// @ts-expect-errors
// @ts-expect-errors For snapshot readability purposes `minimizer` attribute doesn't match the correct type
minimizer: ['banana'],
},
};
Expand All @@ -70,8 +67,7 @@ describe('mergeConfigs', () => {

it('merges partial custom config', () => {
const customConfig: Configuration = {
// For snapshot readability purposes `plugins` attribute doesn't match the correct type
// @ts-expect-errors
// @ts-expect-errors For snapshot readability purposes `plugins` attribute doesn't match the correct type
plugins: ['p3'],
resolve: {
extensions: ['.ts', '.tsx'],
Expand Down
16 changes: 2 additions & 14 deletions code/lib/preview-web/src/render/StoryRender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,6 @@ export type RenderPhase =
| 'aborted'
| 'errored';

function createController(): AbortController {
if (AbortController) return new AbortController();
// Polyfill for IE11
return {
signal: { aborted: false },
abort() {
// @ts-ignore (should be @ts-expect-error but fails build --prep)
this.signal.aborted = true;
},
} as AbortController;
}

function serializeError(error: any) {
try {
const { name = 'Error', message = String(error), stack } = error;
Expand Down Expand Up @@ -88,7 +76,7 @@ export class StoryRender<TFramework extends AnyFramework> implements Render<TFra
public viewMode: ViewMode,
story?: Story<TFramework>
) {
this.abortController = createController();
this.abortController = new AbortController();

// Allow short-circuiting preparing if we happen to already
// have the story (this is used by docs mode)
Expand Down Expand Up @@ -173,7 +161,7 @@ export class StoryRender<TFramework extends AnyFramework> implements Render<TFra
// render could conceivably still be running after this call.
// We might want to change that in the future.
this.cancelRender();
this.abortController = createController();
this.abortController = new AbortController();
}

// We need a stable reference to the signal -- if a re-mount happens the
Expand Down
2 changes: 1 addition & 1 deletion code/renderers/react/template/stories/errors.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const StoryIsUnrenderable = {
export const StoryContainsUnrenderable = {
render: () => (
<div>
{/* @ts-expect-error */}
{/* @ts-expect-error we're doing it wrong here on purpose */}
<BadComponent />
</div>
),
Expand Down
10 changes: 10 additions & 0 deletions code/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7789,6 +7789,7 @@ __metadata:
"@storybook/semver": ^7.3.2
"@storybook/store": 7.0.0-alpha.33
"@storybook/theming": 7.0.0-alpha.33
"@types/case-sensitive-paths-webpack-plugin": ^2.1.6
"@types/node": ^14.0.10 || ^16.0.0
"@types/terser-webpack-plugin": ^5.2.0
"@types/webpack-dev-middleware": ^5.3.0
Expand Down Expand Up @@ -10149,6 +10150,15 @@ __metadata:
languageName: node
linkType: hard

"@types/case-sensitive-paths-webpack-plugin@npm:^2.1.6":
version: 2.1.6
resolution: "@types/case-sensitive-paths-webpack-plugin@npm:2.1.6"
dependencies:
"@types/webpack": ^4
checksum: 9f379718393f04937be69e742f9b67e346ecca5badb37a02eb1b740a4f37e1c52033b3a9fef04b82e31b9f58ae72ae02cf1027a57ecd969ff5e1b4e6796375d6
languageName: node
linkType: hard

"@types/chai-as-promised@npm:^7.1.2":
version: 7.1.5
resolution: "@types/chai-as-promised@npm:7.1.5"
Expand Down
2 changes: 1 addition & 1 deletion scripts/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module.exports = {
root: true,
extends: ['@storybook/eslint-config-storybook', 'plugin:storybook/recommended'],
rules: {
'@typescript-eslint/ban-ts-comment': 'warn',
'@typescript-eslint/ban-ts-comment': 'error',
'jest/no-standalone-expect': [
'error',
{ additionalTestBlockFunctions: ['it.skipWindows', 'it.onWindows'] },
Expand Down