Skip to content

Commit

Permalink
Fix ESLint (#188)
Browse files Browse the repository at this point in the history
* fix 'lint' npm script and add as CI step

The 'lint' npm script as originally written wasn't working because a path was never specified.  (a file or folder path is required).

Additionally, it was not linting typescript files because .ts and .tsx are not checked by default.  This enables those extensions as well.

Finally, I added a lint step to the GA build

* remove extra eslintrc.json file

While trying to fix issue #175 where VSCode was showing a red squiggly for value TypeScript, I discovered that we have two eslintrc.json files, where in fact we should only need one.  Removing this eslintrc file which specified a babel parser, exposes actual TS lint errors which could not be seen before.

Those errors will be resolved in a future commit.

* Auto fix ESLint errors on save in VSCode

Change the VSCode workspace settings to fix ESLint errors that can be automatically fixed.

* specify eslint env as browser for react

As it turns out, we do need a separate eslintrc.json for the react, and this environment setting seems to be the only thing needed at this point.

See https://stackoverflow.com/questions/41858052/solving-linter-error-no-undef-for-document for more information

* eslint: rename IAnalyticsContext interface

* eslint typescript auto fix errors

The following changes were automatically fixed using eslint --fix

* don't auto fix in npm lint script

I decided against auto fixing linter errors in the npm script as this step is run in GA CI, which is not the place to automatically fix anything.

In VSCode at least, auto fixing linter errors should happen automatically on file save.

* Fix failing CI lint

Co-authored-by: Ira Hopkinson <ira_hopkinson@sil.org>
  • Loading branch information
megahirt and irahopkinson authored Aug 3, 2021
1 parent 62a58eb commit 411f8ad
Show file tree
Hide file tree
Showing 16 changed files with 48 additions and 58 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ jobs:
with:
node-version: ${{ matrix.node_version }}
- run: npm ci
- run: npm run lint
- run: npm run build
env:
# ToDo: remove this.
Expand Down
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
},
"editor.rulers": [120],
"editor.wordWrapColumn": 120,
"eslint.validate": ["typescript", "typescriptreact"],
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"preelectron-pack-mac": "npm run build",
"preelectron-pack-win": "npm run build",
"preelectron-pack-linux": "npm run build",
"lint": "eslint -c .eslintrc.json --ignore-path .eslintignore --cache --fix",
"lint": "eslint --ext ts,tsx --cache src public",
"prettier": "prettier --write \"**/*.{ts,js,json,css,scss,less,html,md,yml}\"",
"prettier:test": "prettier --list-different \"**/*.{ts,js,json,css,scss,less,html,md,yml}\"",
"electron-pack-mac": "electron-builder -m",
Expand Down
2 changes: 1 addition & 1 deletion public/models/timings.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface LineTiming {
readonly content: string;
readonly text: string; // not used
readonly words: WordTiming[];
readonly isHeading: Boolean;
readonly isHeading: boolean;
}

interface WordTiming {
Expand Down
17 changes: 2 additions & 15 deletions src/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
{
"env": {
"browser": true,
"commonjs": false,
"es6": true
},
"extends": "../.eslintrc.json",
"parser": "babel-eslint",
"parserOptions": {
"ecmaFeatures": {
"jsx": true
},
"ecmaVersion": 2018,
"sourceType": "module"
},
"plugins": ["react"],
"rules": {}
"browser": true
}
}
2 changes: 1 addition & 1 deletion src/App/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export default class Analytics implements AnalyticsInterface {
}
}

async trackEvent(category: string, action: string, label: string = '', value: number = 0): Promise<void> {
async trackEvent(category: string, action: string, label = '', value = 0): Promise<void> {
const params = { ec: category, ea: action, el: label, ev: value };
if (this.isEnabled && this.ga) {
await this.ga.send('event', params);
Expand Down
2 changes: 1 addition & 1 deletion src/App/components/Actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const ProgressText = (prop: { progress: Progress }): JSX.Element => {
// ));
};

const Action = (prop: { icon: IconName | MaybeElement, intent: Intent, disabled: boolean, onClick: (event: React.MouseEvent<HTMLElement>) => void, combined: boolean, mainText: string, subText: string }): JSX.Element => {
const Action = (prop: { icon: IconName | MaybeElement; intent: Intent; disabled: boolean; onClick: (event: React.MouseEvent<HTMLElement>) => void; combined: boolean; mainText: string; subText: string }): JSX.Element => {
const { appState } = useStores();
return useObserver(() => {
const progress: Progress = appState.progress.combined === prop.combined ? appState.progress : null;
Expand Down
10 changes: 5 additions & 5 deletions src/App/components/Analytics.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import { Text } from '../blueprint';
import { AnalyticsInterface } from '../../../public/models/analytic.model';
import styled from 'styled-components';

export interface IAnalyticsContext {
analytics: AnalyticsInterface
export interface AnalyticsContext {
analytics: AnalyticsInterface;
}

const analyticsContext = React.createContext<IAnalyticsContext>({ analytics: new Analytics({ enableAnalytics: true }) });
const analyticsContext = React.createContext<AnalyticsContext>({ analytics: new Analytics({ enableAnalytics: true }) });

interface AnalyticsProviderSettings {
enableAnalytics: boolean;
Expand All @@ -21,7 +21,7 @@ const StyleText = styled(Text).attrs({
mb: 2,
})``;

export function AnalyticsProvider(prop: {settings: AnalyticsProviderSettings, children: JSX.Element[] | JSX.Element}): JSX.Element {
export function AnalyticsProvider(prop: {settings: AnalyticsProviderSettings; children: JSX.Element[] | JSX.Element}): JSX.Element {
const [analyticsNoticeDisplayed, setAnalyticsNoticeDisplayed] = React.useState(localStorage.analyticsNoticeDisplayed);
const [analytics,] = React.useState<AnalyticsInterface>(new Analytics(prop.settings));

Expand Down Expand Up @@ -59,6 +59,6 @@ AnalyticsProvider.propTypes = {
children: PropTypes.node,
};

export function useAnalytics(): IAnalyticsContext {
export function useAnalytics(): AnalyticsContext {
return React.useContext(analyticsContext);
}
2 changes: 1 addition & 1 deletion src/App/components/AnimatedVisibility.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const Wrapper = styled(Box)`
`;

interface AnimatedVisibilityProps {
visible: boolean,
visible: boolean;
children: JSX.Element[] | JSX.Element;
}

Expand Down
13 changes: 6 additions & 7 deletions src/App/components/ColorPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import styled from 'styled-components';
import { Box } from 'reflexbox';
import { Popover } from '@blueprintjs/core';
import { Color, ColorChangeHandler, ColorResult, SketchPicker } from 'react-color';
import { isConstructorTypeNode } from 'typescript';

const SWATCH_COLORS = [
'#ff3b30',
Expand Down Expand Up @@ -45,12 +44,12 @@ const Swatch = styled(Box).attrs({
`;

interface ColorPickerSettings {
value?: Color,
presetColors?: { color: string; title: string }[] | string[],
disableAlpha?: boolean,
disabled?: boolean,
onChange?: ColorChangeHandler,
props?: any
value?: Color;
presetColors?: { color: string; title: string }[] | string[];
disableAlpha?: boolean;
disabled?: boolean;
onChange?: ColorChangeHandler;
props?: any;
}

export default class ColorPicker extends React.Component<ColorPickerSettings> {
Expand Down
8 changes: 4 additions & 4 deletions src/App/components/Editors/EditPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export const EditRow = styled(Flex).attrs({
})``;

interface PopoverProps {
icon?: IconName | MaybeElement,
title?: string | JSX.Element,
children?: JSX.Element,
props?: any[]
icon?: IconName | MaybeElement;
title?: string | JSX.Element;
children?: JSX.Element;
props?: any[];
}

export default function EditPopover( { icon = "annotation", title, children, ...props }: PopoverProps): JSX.Element {
Expand Down
22 changes: 11 additions & 11 deletions src/App/components/FileSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,22 @@ import { remote, OpenDialogOptions, SaveDialogOptions } from 'electron';
const { dialog } = remote;

const FileSelector = (prop: {
save?: boolean,
buttonText?: React.ReactNode,
buttonIcon?: IconName | MaybeElement,
disabled?: boolean,
file?: string,
options: SaveDialogOptions | OpenDialogOptions,
onFileSelected: (file: string) => void,
}) => {
const selectFile = async () => {
let filePath: string = "";
save?: boolean;
buttonText?: React.ReactNode;
buttonIcon?: IconName | MaybeElement;
disabled?: boolean;
file?: string;
options: SaveDialogOptions | OpenDialogOptions;
onFileSelected: (file: string) => void;
}): JSX.Element => {
const selectFile = async (): Promise<void> => {
let filePath = '';

if (prop.save) {
filePath = (await dialog.showSaveDialog(prop.options as SaveDialogOptions)).filePath || '';
}
else {
let filePaths = (await dialog.showOpenDialog(prop.options as OpenDialogOptions)).filePaths;
const filePaths = (await dialog.showOpenDialog(prop.options as OpenDialogOptions)).filePaths;
filePath = filePaths && filePaths.length === 1 ? filePaths[0] : '';
}

Expand Down
4 changes: 2 additions & 2 deletions src/App/components/Preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const PreviewWord = styled.div`
padding: 0 5px;
margin: 0 -5px;
display: inline-block;
${(prop: { isHighlighted: boolean, highlightColor: string }): string => {
${(prop: { isHighlighted: boolean; highlightColor: string }): string => {
return `background-color: ${prop.isHighlighted ? prop.highlightColor || 'transparent' : 'transparent'};`;
}}
`;
Expand All @@ -99,7 +99,7 @@ const getImageSrc = _.memoize((file: string): string => {
return '';
});

const PreviewVerse = (prop: { verse: string, highlightVerse: boolean, highlightColor: string }): JSX.Element => {
const PreviewVerse = (prop: { verse: string; highlightVerse: boolean; highlightColor: string }): JSX.Element => {
return <>{prop.verse.split(' ').map((word, index) => {
const isHighlighted = prop.highlightVerse && HIGHLIGHT_WORD_INDEXES.includes(index);
return (
Expand Down
4 changes: 2 additions & 2 deletions src/App/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import ChapterSelector from './components/ChapterSelector';
import Preview from './components/Preview';
import Actions from './components/Actions';
import { useStores } from './store';
import { IAnalyticsContext, useAnalytics } from './components/Analytics';
import { AnalyticsContext, useAnalytics } from './components/Analytics';
import './index.scss';

const AppWrapper: StyledComponent<BoxType, any, {}> = styled(Flex)`
Expand All @@ -20,7 +20,7 @@ const AppWrapper: StyledComponent<BoxType, any, {}> = styled(Flex)`

export default function App(): JSX.Element {
const storeRecord: Record<string, any> = useStores();
const analyticsContext: IAnalyticsContext = useAnalytics();
const analyticsContext: AnalyticsContext = useAnalytics();

React.useEffect((): void => {
ipcRenderer.send('did-start-getprojectstructure', storeRecord.settings.rootDirectories);
Expand Down
12 changes: 6 additions & 6 deletions src/App/store/AppState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ const SAMPLE_VERSES = [
'God called the light Day, and the darkness he called Night. And there was evening and there was morning, the first day.',
];

const list = <T = any>(dict: { [name: string]: T }, sortKey: string = 'name'): T[] => _.sortBy(_.values(dict), sortKey);
const list = <T = any>(dict: { [name: string]: T }, sortKey = 'name'): T[] => _.sortBy(_.values(dict), sortKey);

const dict = <T = any>(list: T[], classType?: { new (item: any): T }, key: string = 'name'): { [name: string]: T } => {
const dict = <T = any>(list: T[], classType?: { new (item: any): T }, key = 'name'): { [name: string]: T } => {
return list.reduce((items, item) => {
items[item[key]] = classType ? new classType(item) : item;
return items;
Expand Down Expand Up @@ -88,7 +88,7 @@ class Chapter {
class Book {
name: string;

constructor({ name, chapters }: { name: string; chapters: Object[] }) {
constructor({ name, chapters }: { name: string; chapters: Record<string, any>[] }) {
this.name = name;
this.chapterList = chapters.map((chapter: any) => new Chapter(chapter));
this.chapters = dict<Chapter>(this.chapterList);
Expand Down Expand Up @@ -130,7 +130,7 @@ class Project {
name: string;
fullPath: string;

constructor({ name, fullPath, books }: { name: string; fullPath: string; books: Object[] }) {
constructor({ name, fullPath, books }: { name: string; fullPath: string; books: Record<string, any>[] }) {
this.name = name;
this.fullPath = fullPath;
this.bookList = books.map((book: any) => new Book(book));
Expand All @@ -155,7 +155,7 @@ class Project {
bookSelection: string[] = [];

@observable
activeBookName: string = '';
activeBookName = '';

@computed({ keepAlive: true })
get selectedBooks(): Book[] {
Expand Down Expand Up @@ -192,7 +192,7 @@ class Project {
}
}

selectionToJS(): Object {
selectionToJS(): Record<string, any> {
return {
name: this.name,
fullPath: this.fullPath,
Expand Down
2 changes: 1 addition & 1 deletion src/App/store/Settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Settings {

@persist
@observable
enableAnalytics: boolean = false;
enableAnalytics = false;

@computed({ keepAlive: true })
get rootDirectories(): { [x: string]: string[] } {
Expand Down

0 comments on commit 411f8ad

Please sign in to comment.