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

chore: add a new eslint rule prevent using margin #17024

Merged
merged 5 commits into from
Dec 20, 2024
Merged
Changes from 1 commit
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
Next Next commit
chore: add a new eslint rule prevent using margin
add a new eslint rule prevent using margin

close AUTH-
koji committed Dec 3, 2024
commit 46ad0837a91003618ec22852caca8b59d984be24
18 changes: 15 additions & 3 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -173,19 +173,31 @@ module.exports = {
files: ['./app/src/**/*.@(ts|tsx)'],
rules: {
'opentrons/no-imports-up-the-tree-of-life': 'error',
'opentrons/no-margins-in-css': 'warn',
'opentrons/no-margins-inline': 'warn',
},
},
{
files: ['./protocol-designer/src/**/*.@(ts|tsx)'],
rules: {
'opentrons/no-imports-up-the-tree-of-life': 'warn',
'opentrons/no-margins-in-css': 'warn',
'opentrons/no-margins-inline': 'warn',
},
},
// apply application structure import requirements to app
{
files: ['./app/src/**/*.@(ts|tsx)'],
files: ['./opentrons-ai-client/src/**/*.@(ts|tsx)'],
rules: {
'opentrons/no-imports-up-the-tree-of-life': 'warn',
'opentrons/no-margins-in-css': 'warn',
'opentrons/no-margins-inline': 'warn',
},
},
{
files: ['./components/src/**/*.@(ts|tsx)'],
rules: {
'opentrons/no-imports-across-applications': 'error',
'opentrons/no-margins-in-css': 'warn',
'opentrons/no-margins-inline': 'warn',
},
},
],
Original file line number Diff line number Diff line change
@@ -8,10 +8,10 @@ import {
Flex,
Icon,
JUSTIFY_SPACE_BETWEEN,
LegacyStyledText,
Link,
SPACING_AUTO,
SPACING,
LegacyStyledText,
TYPOGRAPHY,
} from '@opentrons/components'

2 changes: 2 additions & 0 deletions scripts/eslint-plugin-opentrons/lib/index.js
Original file line number Diff line number Diff line change
@@ -4,4 +4,6 @@
module.exports.rules = {
'no-imports-up-the-tree-of-life': require('./rules/no-imports-up-the-tree-of-life'),
'no-imports-across-applications': require('./rules/no-imports-across-applications'),
'no-margins-in-css': require('./rules/no-margins-in-css'),
'no-margins-inline': require('./rules/no-margins-inline'),
}
52 changes: 52 additions & 0 deletions scripts/eslint-plugin-opentrons/lib/rules/no-margins-in-css.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
module.exports = {
meta: {
type: 'problem',
docs: {
description: 'Disallow the use of margin-related properties in css-in-js',
category: 'Best Practices',
recommended: false,
},
messages: {
noMarginInCssInJs: "Avoid using '{{property}}' in css-in-js.",
},
},
create(context) {
return {
// Check for CSS-in-JS template literals
TaggedTemplateExpression(node) {
const forbiddenMargins = [
'margin',
'marginTop',
'marginLeft',
'marginRight',
'marginBottom',
'marginX',
'marginY',
'margin-top',
'margin-left',
'margin-right',
'margin-bottom',
]

if (node.tag.type === 'Identifier' && node.tag.name === 'css') {
const templateLiteral = node.quasi
templateLiteral.quasis.forEach(quasi => {
const text = quasi.value.raw
forbiddenMargins.forEach(property => {
const regex = new RegExp(`\\b${property}\\b`, 'i')
if (regex.test(text)) {
context.report({
node: quasi,
messageId: 'noMarginInCssInJs',
data: {
property,
},
})
}
})
})
}
},
}
},
}
50 changes: 50 additions & 0 deletions scripts/eslint-plugin-opentrons/lib/rules/no-margins-inline.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
module.exports = {
meta: {
type: 'problem',
docs: {
description: 'Disallow the use of margin-related styles',
category: 'Best Practices',
recommended: false,
},
messages: {
noMarginInline: "Avoid using '{{property}}' in your components.",
},
schema: [],
},
create(context) {
const forbiddenMargins = [
'margin',
'marginLeft',
'marginRight',
'marginTop',
'marginBottom',
]

return {
// Existing visitor for object properties
Property(node) {
if (forbiddenMargins.includes(node.key.name || node.key.value)) {
context.report({
node: node.key,
messageId: 'noMarginInline',
data: {
property: node.key.name || node.key.value,
},
})
}
},
// New visitor for JSX attributes
JSXAttribute(node) {
if (forbiddenMargins.includes(node.name.name)) {
context.report({
node: node.name,
messageId: 'noMarginInline',
data: {
property: node.name.name,
},
})
}
},
}
},
}

Unchanged files with check annotations Beta

units?: string
type?: string
}
interface PipetteQuirksField {

Check warning on line 62 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 62 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
[quirkId: string]: boolean
}
interface QuirksField {
quirks?: PipetteQuirksField
}
export type PipetteSettingsFieldsMap = QuirksField & {

Check warning on line 69 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 69 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
[fieldId: string]: PipetteSettingsField
}
export interface IndividualPipetteSettings {
fields: PipetteSettingsFieldsMap
}
type PipetteSettingsById = Partial<{ [id: string]: IndividualPipetteSettings }>

Check warning on line 77 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 77 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
export type PipetteSettings = PipetteSettingsById
export interface PipetteSettingsUpdateFieldsMap {

Check warning on line 81 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 81 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
[fieldId: string]: PipetteSettingsUpdateField
}
} | null
export interface UpdatePipetteSettingsData {
fields: { [fieldId: string]: PipetteSettingsUpdateField }

Check warning on line 90 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 90 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
}
export interface ResourceLink {
href: string
meta?: Partial<{ [key: string]: string | null | undefined }>

Check warning on line 14 in api-client/src/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 14 in api-client/src/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
}
export type ResourceLinks = Record<
export const appRestart = (message: string): AppRestartAction => ({
type: APP_RESTART,
payload: {
message: message,

Check warning on line 360 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 360 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})
export const reloadUi = (message: string): ReloadUiAction => ({
type: RELOAD_UI,
payload: {
message: message,

Check warning on line 368 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 368 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})
export const sendLog = (message: string): SendLogAction => ({
type: SEND_LOG,
payload: {
message: message,

Check warning on line 376 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 376 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})
export const updateBrightness = (message: string): UpdateBrightnessAction => ({
type: UPDATE_BRIGHTNESS,
payload: {
message: message,

Check warning on line 384 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 384 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})