Skip to content

Commit

Permalink
Add eslint rules (#577)
Browse files Browse the repository at this point in the history
- add eslint rule no-unnecessary-type-assertion
- add eslint rule no-type-assertion
- remove no-non-null-assertion covered by no-non-null-assertion
- also wrapped some overlength comments

Note:
- Don't need a reason to excuse using `as` in tests since you often need to spoof data.
- Most changes are repetative adding of comments to excuse using `as`.
- Some changes remove the need for using `as`.
- One potential bug was found and noted a TODO.
  • Loading branch information
irahopkinson authored Oct 19, 2023
1 parent 9081ac1 commit 1ffa861
Show file tree
Hide file tree
Showing 58 changed files with 714 additions and 272 deletions.
5 changes: 3 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ module.exports = {
},
],
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-non-null-assertion': 'error',
'no-redeclare': 'off',
'@typescript-eslint/no-redeclare': 'error',
'no-shadow': 'off',
Expand All @@ -55,6 +54,7 @@ module.exports = {
'error',
{ functions: false, allowNamedExports: true, typedefs: false, ignoreTypeReferences: true },
],
'@typescript-eslint/no-unnecessary-type-assertion': 'error',
'no-unused-vars': 'off',
'@typescript-eslint/no-unused-vars': 'error',
'no-useless-constructor': 'off',
Expand All @@ -70,6 +70,7 @@ module.exports = {
// Should use our logger anytime you want logs that persist. Otherwise use console only in testing
'no-console': 'warn',
'no-plusplus': ['error', { allowForLoopAfterthoughts: true }],
'no-type-assertion/no-type-assertion': 'error',
'prettier/prettier': ['warn', { tabWidth: 2, trailingComma: 'all' }],
'react/jsx-indent-props': ['warn', 2],
'react/jsx-props-no-spreading': ['error', { custom: 'ignore' }],
Expand All @@ -87,7 +88,7 @@ module.exports = {
tsconfigRootDir: __dirname,
createDefaultProgram: true,
},
plugins: ['@typescript-eslint'],
plugins: ['@typescript-eslint', 'no-type-assertion'],
settings: {
'import/resolver': {
// See https://github.com/benmosher/eslint-plugin-import/issues/1396#issuecomment-575727774 for line below
Expand Down
4 changes: 2 additions & 2 deletions .storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ const config: StorybookConfig = {
return mergeWithCustomize({
customizeArray(wpModule: object[], rModule: object[], moduleKey: string) {
if (moduleKey === 'rules') {
const wpRules = wpModule as RuleSetRule[];
const rRules = rModule as RuleSetRule[];
const wpRules: RuleSetRule[] = wpModule;
const rRules: RuleSetRule[] = rModule;
return [
...wpRules.filter(
(rule) =>
Expand Down
5 changes: 5 additions & 0 deletions extensions/src/hello-someone/hello-someone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,14 @@ const peopleDataProviderEngine: IDataProviderEngine<PeopleDataTypes> &
*/
getPerson<T extends boolean = true>(
name: string,
// Assert the conditional type.
// eslint-disable-next-line no-type-assertion/no-type-assertion
createIfDoesNotExist: T = true as T,
): T extends true ? Person : Person | undefined {
const nameLower = name.toLowerCase();
if (createIfDoesNotExist && !this.people[nameLower]) this.people[nameLower] = {};
// Type assert because we know this person exists
// eslint-disable-next-line no-type-assertion/no-type-assertion
return this.people[nameLower] as T extends true ? Person : Person | undefined;
},

Expand Down Expand Up @@ -238,6 +241,8 @@ const peopleWebViewProvider: IWebViewProvider = {
return {
...savedWebView,
title: 'People',
// Can't use the enum value from a definition file so assert the type from the string literal.
// eslint-disable-next-line no-type-assertion/no-type-assertion
contentType: 'html' as WebViewContentType.HTML,
content: helloSomeoneHtmlWebView,
};
Expand Down
2 changes: 2 additions & 0 deletions extensions/src/hello-world/hello-world.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const htmlWebViewProvider: IWebViewProviderWithType = {
return {
...savedWebView,
title: 'Hello World HTML',
// Can't use the enum value from a definition file so assert the type from the string literal.
// eslint-disable-next-line no-type-assertion/no-type-assertion
contentType: 'html' as WebViewContentType.HTML,
content: helloWorldHtmlWebView,
};
Expand Down
7 changes: 3 additions & 4 deletions extensions/src/hello-world/web-views/hello-world.web-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ globalThis.webViewComponent = function HelloWorld() {
iconUrl: 'papi-extension://hello-world/assets/offline.svg',
title: 'Select Hello World Project',
}).current,
// Assert as string type rather than string literal type.
// eslint-disable-next-line no-type-assertion/no-type-assertion
'None' as DialogTypes['platform.selectProject']['responseType'],
);

Expand Down Expand Up @@ -191,10 +193,7 @@ globalThis.webViewComponent = function HelloWorld() {
<Switch /> {/* no label available */}
<ComboBox title="Test Me" options={['option 1', 'option 2']} />
<Slider /> {/* no label available */}
<RefSelector
scrRef={scrRef as ScriptureReference}
handleSubmit={(newScrRef) => setScrRef(newScrRef)}
/>
<RefSelector scrRef={scrRef} handleSubmit={(newScrRef) => setScrRef(newScrRef)} />
<Table<Row>
columns={[
{
Expand Down
10 changes: 5 additions & 5 deletions extensions/webpack/webpack.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ export function getWebViewTempPath(
*/
export async function getWebViewEntries(): Promise<webpack.EntryObject> {
const tsxWebViews = await getWebViewTsxPaths();
const webViewEntries = Object.fromEntries(
const webViewEntries: webpack.EntryObject = Object.fromEntries(
tsxWebViews.map((webViewPath) => [
webViewPath,
{
import: webViewPath,
filename: getWebViewTempPath(webViewPath),
} as webpack.EntryObject[string],
},
]),
);
return webViewEntries;
Expand Down Expand Up @@ -191,7 +191,7 @@ export function getMainEntries(extensions: ExtensionInfo[]): webpack.EntryObject
library: {
type: LIBRARY_TYPE,
},
} as webpack.EntryObject[string],
},
]),
);
return mainEntries;
Expand Down Expand Up @@ -257,9 +257,9 @@ export async function getExtensions(): Promise<ExtensionInfo[]> {
path.join(sourceFolder, extensionFolderName, 'manifest.json'),
'utf8',
);
const extensionManifest = Object.freeze({
const extensionManifest: Readonly<ExtensionManifest> = Object.freeze({
// Note that this does not transform the main file .ts into .js unlike extension.service
...(JSON.parse(extensionManifestJson) as ExtensionManifest),
...JSON.parse(extensionManifestJson),
});

// Get main file path from the manifest and return extension info
Expand Down
28 changes: 20 additions & 8 deletions lib/papi-dts/papi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1057,8 +1057,10 @@ declare module 'shared/services/network.service' {
* Register a local request handler to run on requests.
* @param requestType the type of request on which to register the handler
* @param handler function to register to run on requests
* @param handlerType type of handler function - indicates what type of parameters and what return type the handler has
* @returns promise that resolves if the request successfully registered and unsubscriber function to run to stop the passed-in function from handling requests
* @param handlerType type of handler function - indicates what type of parameters and what return
* type the handler has
* @returns promise that resolves if the request successfully registered and unsubscriber function
* to run to stop the passed-in function from handling requests
*/
export function registerRequestHandler(
requestType: SerializedRequestType,
Expand Down Expand Up @@ -1096,7 +1098,8 @@ declare module 'shared/services/network.service' {
* Creates a function that is a request function with a baked requestType.
* This is also nice because you get TypeScript type support using this function.
* @param requestType requestType for request function
* @returns function to call with arguments of request that performs the request and resolves with the response contents
* @returns function to call with arguments of request that performs the request and resolves with
* the response contents
*/
export const createRequestFunction: <TParam extends unknown[], TReturn>(
requestType: SerializedRequestType,
Expand Down Expand Up @@ -2131,7 +2134,12 @@ declare module 'shared/services/web-view.service' {
*
* Not exposed on the papi
*/
export const addTab: (savedTabInfo: SavedTabInfo, layout: Layout) => Promise<Layout | undefined>;
export const addTab: <TData = unknown>(
savedTabInfo: SavedTabInfo & {
data?: TData | undefined;
},
layout: Layout,
) => Promise<Layout | undefined>;
/**
* Creates a new web view or gets an existing one depending on if you request an existing one and
* if the web view provider decides to give that existing one to you (it is up to the provider).
Expand Down Expand Up @@ -2442,6 +2450,7 @@ declare module 'shared/services/data-provider.service' {
export default dataProviderService;
}
declare module 'shared/models/project-metadata.model' {
import { ProjectTypes } from 'papi-shared-types';
/**
* Low-level information describing a project that Platform.Bible directly manages and uses to load project data
*/
Expand All @@ -2461,7 +2470,7 @@ declare module 'shared/models/project-metadata.model' {
/**
* Indicates what sort of project this is which implies its data shape (e.g., what data streams should be available)
*/
projectType: string;
projectType: ProjectTypes;
};
}
declare module 'shared/services/project-lookup.service-model' {
Expand Down Expand Up @@ -2801,22 +2810,25 @@ declare module 'shared/services/settings.service' {
/**
* Retrieves the value of the specified setting
* @param key The string id of the setting for which the value is being retrieved
* @returns The value of the specified setting, parsed to an object. Returns `null` if setting is not present or no value is available
* @returns The value of the specified setting, parsed to an object. Returns `null` if setting is
* not present or no value is available
*/
const getSetting: <SettingName extends keyof SettingTypes>(
key: SettingName,
) => Nullable<SettingTypes[SettingName]>;
/**
* Sets the value of the specified setting
* @param key The string id of the setting for which the value is being retrieved
* @param newSetting The value that is to be stored. Setting the new value to `null` is the equivalent of deleting the setting
* @param newSetting The value that is to be stored. Setting the new value to `null` is the
* equivalent of deleting the setting
*/
const setSetting: <SettingName extends keyof SettingTypes>(
key: SettingName,
newSetting: Nullable<SettingTypes[SettingName]>,
) => void;
/**
* Subscribes to updates of the specified setting. Whenever the value of the setting changes, the callback function is executed.
* Subscribes to updates of the specified setting. Whenever the value of the setting changes, the
* callback function is executed.
* @param key The string id of the setting for which the value is being subscribed to
* @param callback The function that will be called whenever the specified setting is updated
* @returns Unsubscriber that should be called whenever the subscription should be deleted
Expand Down
Loading

0 comments on commit 1ffa861

Please sign in to comment.