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

update GraphQL-ESLint to v4 in Schema Policies #6101

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
22 changes: 10 additions & 12 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ const guildConfig = require('@theguild/eslint-config/base');
const { REACT_RESTRICTED_SYNTAX, RESTRICTED_SYNTAX } = require('@theguild/eslint-config/constants');
const path = require('path');

const SCHEMA_PATH = './packages/services/api/src/modules/*/module.graphql.ts';
const OPERATIONS_PATHS = [
'./packages/web/app/**/*.ts',
'./packages/web/app/**/*.tsx',
'./packages/web/app/**/*.graphql',
];

const rulesToExtends = Object.fromEntries(
Object.entries(guildConfig.rules).filter(([key]) =>
[
Expand Down Expand Up @@ -70,20 +63,25 @@ module.exports = {
parser: '@graphql-eslint/eslint-plugin',
plugins: ['@graphql-eslint'],
parserOptions: {
schema: SCHEMA_PATH,
operations: OPERATIONS_PATHS,
graphQLConfig: {
schema: './packages/services/api/src/modules/*/module.graphql.ts',
documents: [
'./packages/web/app/**/*.ts',
'./packages/web/app/**/*.tsx',
'./packages/web/app/**/*.graphql',
],
},
},
},
{
// Setup processor for operations/fragments definitions on code-files
files: ['packages/web/app/**/*.tsx', 'packages/web/app/**/*.ts'],
files: ['packages/web/app/**/*.{ts,tsx}'],
processor: '@graphql-eslint/graphql',
},
{
files: ['packages/web/app/**/*.graphql'],
plugins: ['@graphql-eslint'],
rules: {
'@graphql-eslint/require-id-when-available': 'error',
'@graphql-eslint/require-selections': 'error',
'@graphql-eslint/no-deprecated': 'error',
},
},
Expand Down
7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@
"@graphql-codegen/typescript-operations": "4.3.1",
"@graphql-codegen/typescript-resolvers": "4.4.0",
"@graphql-codegen/urql-introspection": "3.0.0",
"@graphql-eslint/eslint-plugin": "3.20.1",
"@graphql-eslint/eslint-plugin": "4.4.0-alpha-20241210124724-37546942474e54378c2b293d843e141891961747",
"@graphql-inspector/cli": "4.0.3",
"@manypkg/get-packages": "2.2.2",
"@next/eslint-plugin-next": "14.2.18",
"@next/eslint-plugin-next": "15.0.4",
"@parcel/watcher": "2.5.0",
"@sentry/cli": "2.38.2",
"@swc/core": "1.9.2",
Expand Down Expand Up @@ -113,8 +113,7 @@
"mjml-core@4.14.0": "patches/mjml-core@4.14.0.patch",
"@apollo/federation@0.38.1": "patches/@apollo__federation@0.38.1.patch",
"@theguild/editor@1.2.5": "patches/@theguild__editor@1.2.5.patch",
"eslint@8.57.1": "patches/eslint@8.57.1.patch",
"@graphql-eslint/eslint-plugin@3.20.1": "patches/@graphql-eslint__eslint-plugin@3.20.1.patch",
"eslint": "patches/eslint.patch",
"got@14.4.4": "patches/got@14.4.4.patch",
"slonik@30.4.4": "patches/slonik@30.4.4.patch",
"@oclif/core@3.26.6": "patches/@oclif__core@3.26.6.patch",
Expand Down
6 changes: 3 additions & 3 deletions packages/services/policy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
"typecheck": "tsc --noEmit"
},
"devDependencies": {
"@graphql-eslint/eslint-plugin": "3.20.1",
"@graphql-eslint/eslint-plugin": "4.4.0-alpha-20241210124724-37546942474e54378c2b293d843e141891961747",
"@hive/service-common": "workspace:*",
"@sentry/node": "7.120.0",
"@sentry/tracing": "7.114.0",
"@trpc/server": "10.45.2",
"@types/eslint": "8.56.12",
"@types/eslint": "9.6.1",
"ajv": "8.17.1",
"dotenv": "16.4.5",
"eslint": "8.57.1",
"eslint": "9.16.0",
"fastify": "4.28.1",
"graphql": "16.9.0",
"pino-pretty": "11.3.0",
Expand Down
8 changes: 4 additions & 4 deletions packages/services/policy/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ export const schemaPolicyApiRouter = t.router({
availableRules: procedure.query(() => {
return RELEVANT_RULES.map(([name, rule]) => ({
name,
description: rule.meta.docs?.description || '',
recommended: rule.meta.docs?.recommended ?? false,
url: rule.meta.docs?.url,
schema: normalizeAjvSchema(rule.meta.schema) as object | null,
description: rule.meta!.docs!.description || '',
recommended: rule.meta!.docs!.recommended ?? false,
url: rule.meta!.docs!.url,
schema: normalizeAjvSchema(rule.meta!.schema),
}));
}),
validateConfig: procedure.input(CONFIG_CHECK_INPUT_VALIDATION).query(() => {
Expand Down
43 changes: 26 additions & 17 deletions packages/services/policy/src/policy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Ajv from 'ajv';
import { Linter } from 'eslint';
import { z, ZodType } from 'zod';
import { GraphQLESLintRule, parseForESLint, rules } from '@graphql-eslint/eslint-plugin';
import { GraphQLESLintRule, parser, rules } from '@graphql-eslint/eslint-plugin/programmatic';
import { RELEVANT_RULES } from './rules';

const ajv = new Ajv({
Expand All @@ -12,24 +12,23 @@ const ajv = new Ajv({
allowMatchingProperties: true,
});
const linter = new Linter();
linter.defineParser('@graphql-eslint/eslint-plugin', { parseForESLint });

for (const [ruleId, rule] of Object.entries(rules)) {
linter.defineRule(ruleId, rule as any);
}

const RULE_LEVEL = z.union([z.number().min(0).max(2), z.enum(['off', 'warn', 'error'])]);
const RULE_LEVEL = z.union([
//
z.number().min(0).max(2),
z.enum(['off', 'warn', 'error']),
]);

type RulemapValidationType = {
type RuleMapValidationType = {
[RuleKey in keyof typeof rules]: ZodType;
};

export function normalizeAjvSchema(
schema: GraphQLESLintRule['meta']['schema'],
): GraphQLESLintRule['meta']['schema'] {
schema: NonNullable<GraphQLESLintRule['meta']>['schema'],
): NonNullable<GraphQLESLintRule['meta']>['schema'] {
if (Array.isArray(schema)) {
if (schema.length === 0) {
return null;
return;
}

return {
Expand All @@ -40,19 +39,19 @@ export function normalizeAjvSchema(
};
}

return schema || null;
return schema;
}

export function createInputValidationSchema() {
return z
.object(
RELEVANT_RULES.reduce((acc, [name, rule]) => {
const schema = normalizeAjvSchema(rule.meta.schema);
const schema = normalizeAjvSchema(rule.meta!.schema);
const validate = schema ? ajv.compile(schema) : null;

return {
...acc,
[name]: z.union([
[`@graphql-eslint/${name}`]: z.union([
z.tuple([RULE_LEVEL]),
z.tuple(
validate
Expand All @@ -77,7 +76,7 @@ export function createInputValidationSchema() {
),
]),
};
}, {} as RulemapValidationType),
}, {} as RuleMapValidationType),
)
.required()
.partial()
Expand All @@ -94,8 +93,18 @@ export async function schemaPolicyCheck(input: {
return linter.verify(
input.source,
{
parser: '@graphql-eslint/eslint-plugin',
parserOptions: { schema: input.schema },
files: ['*.graphql'],
plugins: {
'@graphql-eslint': { rules },
},
languageOptions: {
parser,
parserOptions: {
graphQLConfig: {
schema: input.schema,
},
},
},
rules: input.policy,
},
'schema.graphql',
Expand Down
47 changes: 21 additions & 26 deletions packages/services/policy/src/rules.ts
Original file line number Diff line number Diff line change
@@ -1,58 +1,53 @@
import { GraphQLESLintRule, rules, type CategoryType } from '@graphql-eslint/eslint-plugin';
import {
GraphQLESLintRule,
rules,
type CategoryType,
} from '@graphql-eslint/eslint-plugin/programmatic';

type AllRulesType = typeof rules;
type RuleName = keyof AllRulesType;

const SKIPPED_RULES: RuleName[] = [
// Skipped because in order to operate, it needs operations.
// Also it does not make sense to run it as part of a schema check.
// Also, it does not make sense to run it as part of a schema check.
'no-unused-fields',
];

function isRelevantCategory(category?: CategoryType | CategoryType[]): boolean {
if (!category) {
return false;
}

return Array.isArray(category) ? category.includes('Schema') : category === 'Schema';
function isRelevantCategory(category: string): boolean {
return (category as CategoryType).includes('schema');
}

// Some rules have configurations for operations (like "alphabetize") and we do not want to expose them.
function patchRulesConfig<T extends RuleName>(
ruleName: T,
ruleDef: AllRulesType[T],
): GraphQLESLintRule {
const { schema } = ruleDef.meta as any;
switch (ruleName) {
case 'alphabetize': {
// Remove operation-specific configurations
delete ruleDef.meta.schema.items.properties.selections;
delete ruleDef.meta.schema.items.properties.variables;
delete schema.items.properties.selections;
delete schema.items.properties.variables;
break;
}
case 'naming-convention': {
// Remove operation-specific configurations
delete ruleDef.meta.schema.items.properties.VariableDefinition;
delete ruleDef.meta.schema.items.properties.OperationDefinition;
delete schema.items.properties.VariableDefinition;
delete schema.items.properties.OperationDefinition;

// Get rid of "definitions" references becuse it's breaking Monaco editor in the frontend
Object.entries(ruleDef.meta.schema.items.properties).forEach(([, propDef]) => {
// Get rid of "definitions" references because it's breaking Monaco editor in the frontend
Object.entries(schema.items.properties).forEach(([, propDef]) => {
if (propDef && typeof propDef === 'object' && 'oneOf' in propDef) {
propDef.oneOf = [
ruleDef.meta.schema.definitions.asObject,
ruleDef.meta.schema.definitions.asString,
];
propDef.oneOf = [schema.definitions.asObject, schema.definitions.asString];
}
});
ruleDef.meta.schema.items.patternProperties = {
schema.items.patternProperties = {
'^(Argument|DirectiveDefinition|EnumTypeDefinition|EnumValueDefinition|FieldDefinition|InputObjectTypeDefinition|InputValueDefinition|InterfaceTypeDefinition|ObjectTypeDefinition|ScalarTypeDefinition|UnionTypeDefinition)(.+)?$':
{
oneOf: [
ruleDef.meta.schema.definitions.asObject,
ruleDef.meta.schema.definitions.asString,
],
oneOf: [schema.definitions.asObject, schema.definitions.asString],
},
};
delete ruleDef.meta.schema.definitions;
delete schema.definitions;

break;
}
Expand All @@ -68,8 +63,8 @@ function patchRulesConfig<T extends RuleName>(
export const RELEVANT_RULES = Object.entries(rules)
.filter(
([ruleName, rule]) =>
isRelevantCategory(rule.meta.docs?.category) &&
rule.meta.docs?.graphQLJSRuleName === undefined &&
isRelevantCategory(rule.meta!.docs!.category!) &&
rule.meta!.docs!.graphQLJSRuleName === undefined &&
!SKIPPED_RULES.includes(ruleName as RuleName),
)
.map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ export const PermissionsSpace = memo(

const UsePermissionManager_OrganizationFragment = graphql(`
fragment UsePermissionManager_OrganizationFragment on Organization {
id
slug
me {
...CanAccessOrganization_MemberFragment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { CurrencyFormatter } from './helpers';

const PriceEstimationTable_PlanFragment = graphql(`
fragment PriceEstimationTable_PlanFragment on BillingPlan {
id
includedOperationsLimit
pricePerOperationsUnit
basePrice
Expand Down Expand Up @@ -72,6 +73,7 @@ function PriceEstimationTable(props: {

const PlanSummary_PlanFragment = graphql(`
fragment PlanSummary_PlanFragment on BillingPlan {
id
planType
retentionInDays
...PriceEstimationTable_PlanFragment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { FragmentType, graphql, useFragment } from '@/gql';

const ProPlanBilling_OrganizationFragment = graphql(`
fragment ProPlanBilling_OrganizationFragment on Organization {
id
billingConfiguration {
hasPaymentIssues
}
Expand Down
2 changes: 2 additions & 0 deletions packages/web/app/src/components/organization/members/list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ const OrganizationMembers_OrganizationFragment = graphql(`
nodes {
id
user {
id
displayName
}
role {
Expand Down Expand Up @@ -626,6 +627,7 @@ export function OrganizationMembers(props: {

const ChangePermissionsModal_OrganizationFragment = graphql(`
fragment ChangePermissionsModal_OrganizationFragment on Organization {
id
...UsePermissionManager_OrganizationFragment
}
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const MemberRoleMigrationStickyNote_OrganizationFragment = graphql(`
id
slug
me {
id
isAdmin
}
unassignedMembersToMigrate {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,14 @@ const ExternalCompositionForm_EnableMutation = graphql(`

const ExternalCompositionForm_OrganizationFragment = graphql(`
fragment ExternalCompositionForm_OrganizationFragment on Organization {
id
slug
}
`);

const ExternalCompositionForm_ProjectFragment = graphql(`
fragment ExternalCompositionForm_ProjectFragment on Project {
id
slug
}
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ const ModelMigrationSettings_upgradeProjectRegistryModelMutation = graphql(`

const ModelMigrationSettings_ProjectFragment = graphql(`
fragment ModelMigrationSettings_ProjectFragment on Project {
id
type
slug
registryModel
Expand Down
4 changes: 2 additions & 2 deletions packages/web/app/src/components/ui/user-menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const UserMenu_OrganizationConnectionFragment = graphql(`
id
slug
me {
id
...UserMenu_MemberFragment
}
getStarted {
Expand All @@ -79,6 +78,7 @@ const UserMenu_MeFragment = graphql(`

const UserMenu_MemberFragment = graphql(`
fragment UserMenu_MemberFragment on Member {
id
canLeaveOrganization
}
`);
Expand Down Expand Up @@ -143,7 +143,7 @@ export function UserMenu(props: {
{me?.provider === AuthProvider.Google ? (
<FaGoogle title="Signed in using Google" />
) : me?.provider === AuthProvider.Github ? (
<FaGithub title="Signed in using Github" />
<FaGithub title="Signed in using GitHub" />
) : (
<FaKey title="Signed in using username and password" />
)}
Expand Down
Loading
Loading