Skip to content

Commit

Permalink
Merge pull request #4670 from Shopify/very-strict-linting
Browse files Browse the repository at this point in the history
Use @typescript-eslint's most robust rules.
  • Loading branch information
shauns authored Oct 18, 2024
2 parents 720bb99 + af1238c commit 2e71328
Show file tree
Hide file tree
Showing 75 changed files with 217 additions and 186 deletions.
30 changes: 29 additions & 1 deletion .github/workflows/shopify-cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ jobs:
matrix:
os: ['ubuntu-latest']
node: ['18.20.3']
target: ['build', 'type-check', 'lint']
target: ['build', 'type-check']
steps:
- uses: actions/checkout@v3
with:
Expand All @@ -127,6 +127,34 @@ jobs:
- name: ${{ matrix.target }}
run: pnpm nx run-many --all --skip-nx-cache --target=${{ matrix.target }} --output-style=stream

pr-platform-agnostic-lint:
name: '[PR] Run lint with Node ${{ matrix.node }} in ${{ matrix.os }}'
runs-on: ${{ matrix.os }}
if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }}
timeout-minutes: 30
strategy:
matrix:
os: ['ubuntu-latest']
node: ['18.20.3']
steps:
- uses: actions/checkout@v3
with:
repository: ${{ github.event.pull_request.head.repo.full_name || github.event.repository.full_name }}
ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }}
fetch-depth: 1
- name: Setup deps
uses: ./.github/actions/setup-cli-deps
with:
node-version: ${{ matrix.node }}
- name: Lint
run: pnpm nx run-many --all --skip-nx-cache --target=lint --output-style=stream --format=json -o eslint-report.json
continue-on-error: true
- name: Annotate Code Linting Results
uses: ataylorme/eslint-annotate-action@21a1ba0738d8b732639999029c4ff40b6e121bb4 # pin@v3
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
report-json: "packages/*/eslint-report.json"

pr-platform-agnostic-bundle:
name: '[PR] Run bundle with Node ${{ matrix.node }} in ${{ matrix.os }}'
runs-on: ${{ matrix.os }}
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,6 @@ testing.mjs

# NX
.nx

# ESLint reports
eslint-report.json
2 changes: 1 addition & 1 deletion packages/app/src/cli/models/app/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export function getAppVersionedSchema(
allowDynamicallySpecifiedConfigs = false,
): ZodObjectOf<Omit<CurrentAppConfiguration, 'path'>> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const schema = specs.reduce((schema, spec) => spec.contributeToAppConfigurationSchema(schema), AppSchema as any)
const schema = specs.reduce<any>((schema, spec) => spec.contributeToAppConfigurationSchema(schema), AppSchema)

if (allowDynamicallySpecifiedConfigs) {
return schema.passthrough()
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
await Promise.all(
['index']
.flatMap((name) => [`${name}.js`, `${name}.jsx`, `${name}.ts`, `${name}.tsx`])
.flatMap((fileName) => [`src/${fileName}`, `${fileName}`])
.flatMap((fileName) => [`src/${fileName}`, fileName])
.map((relativePath) => joinPath(directory, relativePath))
.map(async (sourcePath) => ((await fileExists(sourcePath)) ? sourcePath : undefined)),
)
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
this.uid = this.configuration.uid ?? randomUUID()

if (this.features.includes('esbuild') || this.type === 'tax_calculation') {
this.outputPath = joinPath(this.directory, 'dist', `${this.outputFileName}`)
this.outputPath = joinPath(this.directory, 'dist', this.outputFileName)
}

if (this.isFunctionExtension) {
Expand Down Expand Up @@ -314,7 +314,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi

async buildForBundle(options: ExtensionBuildOptions, bundleDirectory: string, identifiers?: Identifiers) {
const extensionId = identifiers?.extensions[this.localIdentifier] ?? this.configuration.uid ?? this.handle
const outputFile = this.isThemeExtension ? '' : joinPath('dist', `${this.outputFileName}`)
const outputFile = this.isThemeExtension ? '' : joinPath('dist', this.outputFileName)

if (this.features.includes('bundling')) {
// Modules that are going to be inclued in the bundle should be built in the bundle directory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function reduceWebhooks(
subscriptions: WebhookSubscription[],
property?: keyof Pick<WebhookSubscription, 'topics' | 'compliance_topics'>,
) {
return subscriptions.reduce((accumulator, subscription) => {
return subscriptions.reduce<WebhookSubscription[]>((accumulator, subscription) => {
const existingSubscription = findSubscription(accumulator, subscription)
if (existingSubscription) {
if (property && subscription?.[property]?.length) {

Check warning on line 83 in packages/app/src/cli/models/extensions/specifications/transform/app_config_webhook.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/models/extensions/specifications/transform/app_config_webhook.ts#L83

[@typescript-eslint/no-unnecessary-condition] Unnecessary optional chain on a non-nullish value.
Expand All @@ -97,5 +97,5 @@ export function reduceWebhooks(
accumulator.push(subscription)
}
return accumulator
}, [] as WebhookSubscription[])
}, [])
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type Config =

export function extensionUuidToHandle(config: Config, allExtensions: ExtensionRegistration[]) {
const uiExtensionHandle = config.ui_extension_handle
if (uiExtensionHandle || 'ui_extension_registration_uuid' in config === false) {
if (uiExtensionHandle || !('ui_extension_registration_uuid' in config)) {

Check warning on line 16 in packages/app/src/cli/models/extensions/specifications/transform/extension_uuid_to_handle.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/models/extensions/specifications/transform/extension_uuid_to_handle.ts#L16

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
return uiExtensionHandle
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ Please check the module path for ${target}`.value,
)
}

if (uniqueTargets.indexOf(target) === -1) {
if (!uniqueTargets.includes(target)) {
uniqueTargets.push(target)
} else {
duplicateTargets.push(target)
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/prompts/init/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ const init = async (options: InitOptions): Promise<InitOutput> => {
}

if (branch) {
selectedUrl += `#${branch}`
selectedUrl = `${selectedUrl}#${branch}`
}

answers.template = selectedUrl || answers.template || defaults.template

Check warning on line 124 in packages/app/src/cli/prompts/init/init.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/prompts/init/init.ts#L124

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const pollAppLogs = async ({jwtToken, cursor, filters}: PollOptions): Pro
}
if (response.status === 401 || response.status === 429 || response.status >= 500) {
return {
errors: [{status: response.status, message: `${errorResponse.errors.join(', ')}`}],
errors: [{status: response.status, message: errorResponse.errors.join(', ')}],
}
} else {
throw new AbortError(`${errorResponse.errors.join(', ')} while fetching app logs`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const Logs: FunctionComponent<LogsProps> = ({pollOptions: {jwtToken, filters}, r
<Text>
<Text color="green">{prefix.logTimestamp} </Text>
<Text color="blueBright">{`${prefix.storeName.split('.')[0]}`} </Text>
<Text color="blueBright">{`${prefix.source}`} </Text>
<Text color="blueBright">{prefix.source} </Text>
<Text color={prefix.status === 'Success' ? 'green' : 'red'}>{prefix.status} </Text>
<Text>{prefix.description}</Text>
</Text>
Expand Down
10 changes: 2 additions & 8 deletions packages/app/src/cli/services/build/theme-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,9 @@ function formatOffenses(offenses: Offense[]): TokenItem {
const codeSnippet = getSnippet(absolutePath, start.line, end.line)

// Ensure enough padding between offenses
const offensePadding = `${index === offenses.length - 1 ? '' : '\n\n'}`
const offensePadding = index === offenses.length - 1 ? '' : '\n\n'

return [
severityToToken(severity),
{bold: `${check}`},
{subdued: `\n${message}`},
`\n\n${codeSnippet}`,
offensePadding,
]
return [severityToToken(severity), {bold: check}, {subdued: `\n${message}`}, `\n\n${codeSnippet}`, offensePadding]
})

return offenseBodies.flat()
Expand Down
8 changes: 4 additions & 4 deletions packages/app/src/cli/services/deploy/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ function generalErrorsSection(
}

function cliErrorsSections(errors: AppDeploySchema['appDeploy']['userErrors'], identifiers: IdentifiersExtensions) {
return errors.reduce((sections, error) => {
return errors.reduce<ErrorCustomSection[]>((sections, error) => {
const field = (error.field ?? ['unknown']).join('.').replace('extension_points', 'extensions.targeting')
const errorMessage = field === 'base' ? error.message : `${field}: ${error.message}`

Expand Down Expand Up @@ -343,12 +343,12 @@ function cliErrorsSections(errors: AppDeploySchema['appDeploy']['userErrors'], i
})

return sections
}, [] as ErrorCustomSection[])
}, [])
}

function partnersErrorsSections(errors: AppDeploySchema['appDeploy']['userErrors']) {
return errors
.reduce((sections, error) => {
.reduce<{title: string | undefined; errorCount: number}[]>((sections, error) => {
const extensionIdentifier = error.details.find(
(detail) => typeof detail.extension_title !== 'undefined',
)?.extension_title
Expand All @@ -365,7 +365,7 @@ function partnersErrorsSections(errors: AppDeploySchema['appDeploy']['userErrors
}

return sections
}, [] as {title: string | undefined; errorCount: number}[])
}, [])
.map((section) => ({
title: section.title,
body: `\n${section.errorCount} error${
Expand Down
6 changes: 4 additions & 2 deletions packages/app/src/cli/services/dev/extension/payload/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ export class ExtensionsPayloadStore extends EventEmitter {
isNewExtensionPointsSchema(foundExtension.extensionPoints) &&
isNewExtensionPointsSchema(rawPayloadExtension.extensionPoints)
) {
const foundExtensionPointsPayloadMap = foundExtension.extensionPoints.reduce((acc, ex) => {
const foundExtensionPointsPayloadMap = foundExtension.extensionPoints.reduce<{
[key: string]: DevNewExtensionPointSchema
}>((acc, ex) => {
return {...acc, [ex.target]: ex}
}, {} as {[key: string]: DevNewExtensionPointSchema})
}, {})

rawPayloadExtension.extensionPoints = deepMergeObjects(
rawPayloadExtension.extensionPoints,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function getExtensionAssetMiddleware({devOptions}: GetExtensionsMiddlewar
})

Check warning on line 86 in packages/app/src/cli/services/dev/extension/server/middlewares.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/dev/extension/server/middlewares.ts#L83-L86

[@typescript-eslint/no-confusing-void-expression] Returning a void expression from a function is forbidden. Please move it before the `return` statement.
}

const buildDirectory = extension.outputPath.replace(`${extension.outputFileName}`, '')
const buildDirectory = extension.outputPath.replace(extension.outputFileName, '')

return fileServerMiddleware(request, response, next, {
filePath: joinPath(buildDirectory, assetPath),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('app-logs-polling', () => {
token: TOKEN,
}

let subscribeToAppLogs: Mock<any, any>
let subscribeToAppLogs: Mock
let developerPlatformClient: DeveloperPlatformClient
let stdout: any
let stderr: any
Expand Down
7 changes: 2 additions & 5 deletions packages/app/src/cli/services/function/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {hyphenate, camelize} from '@shopify/cli-kit/common/string'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {exec} from '@shopify/cli-kit/node/system'
import {joinPath} from '@shopify/cli-kit/node/path'
import {build as esBuild, BuildResult, BuildOptions} from 'esbuild'
import {build as esBuild, BuildResult} from 'esbuild'
import {findPathUp, inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs'
import {AbortSignal} from '@shopify/cli-kit/node/abort'
import {renderTasks} from '@shopify/cli-kit/node/ui'
Expand Down Expand Up @@ -192,10 +192,7 @@ export async function installJavy(app: AppInterface) {
}

export interface JavyBuilder {
bundle(
fun: ExtensionInstance<FunctionConfigType>,
options: JSFunctionBuildOptions,
): Promise<BuildResult<BuildOptions>>
bundle(fun: ExtensionInstance<FunctionConfigType>, options: JSFunctionBuildOptions): Promise<BuildResult>
compile(fun: ExtensionInstance<FunctionConfigType>, options: JSFunctionBuildOptions): Promise<void>
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ async function runFunctionRunnerWithLogInput(
let functionRunnerOutput = ''
const customStdout = new Writable({
write(chunk, _encoding, next) {
functionRunnerOutput += chunk
functionRunnerOutput += chunk as string
next()
},
})
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function formatSuccessfulRunMessage(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
options.nextSteps!.push([
'To preview this extension along with the rest of the project, run',
{command: `${formatPackageManagerCommand(depndencyManager, 'shopify app dev')}`},
{command: formatPackageManagerCommand(depndencyManager, 'shopify app dev')},
])
}

Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/cli/services/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ class AppInfo {
if (relevantExtensions[0]) {
body += `\n\n${outputContent`${outputToken.subheading(relevantExtensions[0].externalType)}`.value}`
relevantExtensions.forEach((extension: TExtension) => {
body += `${outputFormatter(extension)}`
body += outputFormatter(extension)
})
}
})
Expand All @@ -193,7 +193,7 @@ class AppInfo {
if (this.app.errors?.isEmpty() === false) {
body += `\n\n${outputContent`${outputToken.subheading('Extensions with errors')}`.value}`
supportedExtensions.forEach((extension) => {
body += `${this.invalidExtensionSubSection(extension)}`
body += this.invalidExtensionSubSection(extension)
})
}
return [title, body]
Expand Down Expand Up @@ -268,6 +268,6 @@ class AppInfo {
['Shell', process.env.SHELL || 'unknown'],

Check warning on line 268 in packages/app/src/cli/services/info.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/info.ts#L268

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
['Node version', process.version],
]
return [title, `${linesToColumns(lines)}`]
return [title, linesToColumns(lines)]
}
}
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/init/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async function init(options: InitOptions) {
const repoUrl = githubRepo.branch ? `${githubRepo.baseURL}#${githubRepo.branch}` : githubRepo.baseURL

await mkdir(templateDownloadDir)
const tasks: Task<unknown>[] = [
const tasks: Task[] = [
{
title: `Downloading template from ${repoUrl}`,
task: async () => {
Expand Down Expand Up @@ -229,7 +229,7 @@ async function init(options: InitOptions) {
{link: {label: 'Shopify docs', url: 'https://shopify.dev'}},
[
'For an overview of commands, run',
{command: `${formatPackageManagerCommand(packageManager, 'shopify app', '--help')}`},
{command: formatPackageManagerCommand(packageManager, 'shopify app', '--help')},
],
],
})
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ function renderAppLogsConfigInfo(
const fileName = configFile ? getAppConfigurationFileName(configFile) : undefined

renderInfo({
headline: `${configFile ? `Using ${fileName} for default values:` : 'Using these settings:'}`,
headline: configFile ? `Using ${fileName} for default values:` : 'Using these settings:',
body,
})
}
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/webhook/trigger-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ function equivalentTopic(passedTopic: string, availableTopics: string[]): string
export function deliveryMethodForAddress(address: string | undefined): string | undefined {
if (!address) return undefined

if (PROTOCOL.PUBSUB.test(address)) {
if (address.startsWith('pubsub:')) {
return DELIVERY_METHOD.PUBSUB
}

if (PROTOCOL.EVENTBRIDGE.test(address)) {
if (address.startsWith('arn:aws:events:')) {
return DELIVERY_METHOD.EVENTBRIDGE
}

Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/utilities/app/http-reverse-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function getProxyServerWebsocketUpgradeListener(
function getProxyServerRequestListener(
rules: {[key: string]: string},
proxy: Server,
): http.RequestListener<typeof http.IncomingMessage, typeof http.ServerResponse> | undefined {
): http.RequestListener | undefined {
return function (req, res) {
const target = match(rules, req)
if (target) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ function getAppVars(
if (isLaunchable) {
return {
org: parseInt(org.id, 10),
title: `${name}`,
title: name,
appUrl: 'https://example.com',
redir: ['https://example.com/api/auth'],
requestedAccessScopes: scopesArray ?? [],
Expand All @@ -183,7 +183,7 @@ function getAppVars(
} else {
return {
org: parseInt(org.id, 10),
title: `${name}`,
title: name,
appUrl: MAGIC_URL,
redir: [MAGIC_REDIRECT_URL],
requestedAccessScopes: scopesArray ?? [],
Expand Down
2 changes: 0 additions & 2 deletions packages/cli-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@
"docs:open": "nx docs:open",
"lint": "nx lint",
"lint:ruby": "nx lint:ruby",
"lint:js": "nx lint:js",
"lint:fix": "nx lint:fix",
"lint:js:fix": "nx lint:js:fix",
"lint:ruby:fix": "nx lint:ruby:fix",
"prepack": "cross-env NODE_ENV=production pnpm nx build && cp ../../README.md README.md",
"test": "nx test",
Expand Down
Loading

0 comments on commit 2e71328

Please sign in to comment.