Skip to content

Commit

Permalink
BREAKING(react-conformance): update test type and add new option `dis…
Browse files Browse the repository at this point in the history
…ableTypeTests` to disables tests that requires type information (#28988)

* disableTypeTests

* chglog

* api

* fix other test

* changelog

* type polish

* fix n*

* type update

* api

* type+comment
  • Loading branch information
YuanboXue-Amber authored Sep 6, 2023
1 parent 42ed56c commit 8582884
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: add option `disableTypeTests` to disable tests that require TypeScript information",
"packageName": "@fluentui/react-conformance",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: internal type update with @fluentui/react-conformance version bump",
"packageName": "@fluentui/react-conformance-griffel",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const parseDocblock = (docblock: string) => {
*/
export const extraConformanceTests: TestObject = {
/** Component has a docblock with 5 to 25 words */
'has-docblock': (componentInfo, testInfo) => {
'has-docblock': (_testInfo, componentInfo) => {
const maxWords = 25;
const minWords = 5;

Expand All @@ -37,7 +37,7 @@ export const extraConformanceTests: TestObject = {
},

/** If the component is a subcomponent, ensure its parent has the subcomponent as static property */
'is-static-property-of-parent': (componentInfo, testInfo) => {
'is-static-property-of-parent': testInfo => {
const { componentPath, displayName, Component } = testInfo;
const componentFolder = componentPath.replace(path.basename(componentPath) + path.extname(componentPath), '');
const dirName = path.basename(componentFolder).replace(path.extname(componentFolder), '');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { IsConformantOptions, ConformanceTest, TestOptions } from '@fluentui/react-conformance';
import type { IsConformantOptions, BaseConformanceTest, TestOptions } from '@fluentui/react-conformance';
import './matchers/index';

export const OVERRIDES_WIN_TEST_NAME = 'make-styles-overrides-win';
Expand Down Expand Up @@ -28,7 +28,7 @@ async function getReactComponent(
* A conformance test for mergeClasses() that ensures that a classname from props is passed as a last param,
* i.e. ensures that user's overrides have higher priority.
*/
export const overridesWin: ConformanceTest = (componentInfo, testInfo) => {
export const overridesWin: BaseConformanceTest = testInfo => {
const testOptions = testInfo.testOptions as
| (TestOptions & { [OVERRIDES_WIN_TEST_NAME]?: OverridesWinTestOptions })
| undefined;
Expand Down
8 changes: 6 additions & 2 deletions packages/react-conformance/etc/react-conformance.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import { render } from '@testing-library/react';
import * as ts from 'typescript';

// @public (undocumented)
export type ConformanceTest<TProps = {}> = (componentInfo: ComponentDoc, testInfo: IsConformantOptions<TProps>, tsProgram: ts.Program) => void;
export type BaseConformanceTest<TProps = {}> = (testInfo: IsConformantOptions<TProps>) => void;

// @public (undocumented)
export type ConformanceTest<TProps = {}> = (testInfo: IsConformantOptions<TProps>, componentInfo: ComponentDoc, tsProgram: ts.Program) => void;

// @public (undocumented)
export function isConformant<TProps = {}>(...testInfo: Partial<IsConformantOptions<TProps>>[]): void;
Expand All @@ -20,6 +23,7 @@ export interface IsConformantOptions<TProps = {}> {
Component: React_2.ComponentType<TProps>;
componentPath: string;
disabledTests?: string[];
disableTypeTests?: boolean;
displayName: string;
elementRefName?: string;
extraTests?: TestObject<TProps>;
Expand All @@ -41,7 +45,7 @@ export interface IsConformantOptions<TProps = {}> {
// @public (undocumented)
export interface TestObject<TProps = {}> {
// (undocumented)
[key: string]: ConformanceTest<TProps>;
[key: string]: BaseConformanceTest<TProps> | ConformanceTest<TProps>;
}

// @public
Expand Down
36 changes: 18 additions & 18 deletions packages/react-conformance/src/defaultTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as _ from 'lodash';
import * as path from 'path';
import { render } from '@testing-library/react';

import { TestObject, IsConformantOptions } from './types';
import { IsConformantOptions, DefaultTestObject } from './types';
import { defaultErrorMessages } from './defaultErrorMessages';
import { ComponentDoc } from 'react-docgen-typescript';
import { getPackagePath, getCallbackArguments, validateCallbackArguments } from './utils/index';
Expand Down Expand Up @@ -32,9 +32,9 @@ function getTargetElement(

/* eslint-disable @typescript-eslint/naming-convention */

export const defaultTests: TestObject = {
export const defaultTests: DefaultTestObject = {
/** Component file exports a valid React element type */
'exports-component': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'exports-component': (testInfo: IsConformantOptions) => {
it(`exports component from file under correct name (exports-component)`, () => {
const { componentPath, Component, displayName } = testInfo;
const componentFile = require(componentPath);
Expand All @@ -52,7 +52,7 @@ export const defaultTests: TestObject = {
},

/** Component file exports a valid React element and can render it */
'component-renders': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'component-renders': (testInfo: IsConformantOptions) => {
it(`renders (component-renders)`, () => {
try {
const { requiredProps, Component, renderOptions } = testInfo;
Expand All @@ -67,7 +67,7 @@ export const defaultTests: TestObject = {
* If functional component: component has a displayName
* Else: component's constructor is a named function and matches displayName
*/
'component-has-displayname': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'component-has-displayname': (testInfo: IsConformantOptions) => {
const { Component } = testInfo;

it(`has a displayName or constructor name (component-has-displayname)`, () => {
Expand All @@ -86,7 +86,7 @@ export const defaultTests: TestObject = {
},

/** Component handles ref */
'component-handles-ref': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'component-handles-ref': (testInfo: IsConformantOptions) => {
it(`handles ref (component-handles-ref)`, () => {
// This test simply verifies that the passed ref is applied to an element *anywhere* in the DOM
const { Component, requiredProps, elementRefName = 'ref', renderOptions } = testInfo;
Expand All @@ -108,7 +108,7 @@ export const defaultTests: TestObject = {
},

/** Component has ref applied to the root component DOM node */
'component-has-root-ref': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'component-has-root-ref': (testInfo: IsConformantOptions) => {
it(`applies ref to root element (component-has-root-ref)`, () => {
const { renderOptions, Component, requiredProps, elementRefName = 'ref', primarySlot = 'root' } = testInfo;

Expand Down Expand Up @@ -153,7 +153,7 @@ export const defaultTests: TestObject = {
* (In the extremely unlikely event that someone has a compelling need for the native functionality
* in the future, it can be added under an `htmlSize` prop.)
*/
'omits-size-prop': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'omits-size-prop': (testInfo: IsConformantOptions, componentInfo: ComponentDoc) => {
const sizeType = componentInfo.props.size?.type?.name;
if (!sizeType || componentInfo.props.htmlSize) {
return;
Expand Down Expand Up @@ -188,7 +188,7 @@ export const defaultTests: TestObject = {
},

/** Component file handles classname prop */
'component-handles-classname': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'component-handles-classname': (testInfo: IsConformantOptions) => {
const { Component, requiredProps, renderOptions } = testInfo;
const testClassName = 'testComponentClassName';
let handledClassName = false;
Expand Down Expand Up @@ -253,10 +253,10 @@ export const defaultTests: TestObject = {
},

/** Component file has assigned and exported static classnames object */
'component-has-static-classnames-object': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'component-has-static-classnames-object': (testInfo: IsConformantOptions) => {
const { componentPath, Component, testOptions = {}, requiredProps, renderOptions } = testInfo;

const componentName = componentInfo.displayName;
const componentName = testInfo.displayName;
const classNamePrefix = testOptions?.['component-has-static-classname']?.prefix ?? 'fui';
const componentClassName = `${classNamePrefix}-${componentName}`;
const exportName = `${componentName[0].toLowerCase()}${componentName.slice(1)}ClassNames`;
Expand Down Expand Up @@ -353,7 +353,7 @@ export const defaultTests: TestObject = {
},

/** Constructor/component name matches filename */
'name-matches-filename': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'name-matches-filename': (testInfo: IsConformantOptions) => {
it(`Component/constructor name matches filename (name-matches-filename)`, () => {
try {
const { componentPath, displayName } = testInfo;
Expand All @@ -367,7 +367,7 @@ export const defaultTests: TestObject = {
},

/** Ensures component is exported at top level allowing `import { Component } from 'packageName'` */
'exported-top-level': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'exported-top-level': (testInfo: IsConformantOptions) => {
if (testInfo.isInternal) {
return;
}
Expand All @@ -385,7 +385,7 @@ export const defaultTests: TestObject = {
},

/** Ensures component has top level file in package/src/componentName */
'has-top-level-file': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'has-top-level-file': (testInfo: IsConformantOptions) => {
if (testInfo.isInternal) {
return;
}
Expand All @@ -403,7 +403,7 @@ export const defaultTests: TestObject = {
},

/** Ensures aria attributes are kebab cased */
'kebab-aria-attributes': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'kebab-aria-attributes': (testInfo: IsConformantOptions, componentInfo: ComponentDoc) => {
it(`uses kebab-case for aria attributes (kebab-aria-attributes)`, () => {
const invalidProps = Object.keys(componentInfo.props).filter(
prop => prop.startsWith('aria') && !/^aria-[a-z]+$/.test(prop),
Expand All @@ -418,7 +418,7 @@ export const defaultTests: TestObject = {

// TODO: Test last word of callback name against list of valid verbs
/** Ensures that components have consistent custom callback names i.e. on[Part][Event] */
'consistent-callback-names': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'consistent-callback-names': (testInfo: IsConformantOptions, componentInfo: ComponentDoc) => {
it(`has consistent custom callback names (consistent-callback-names)`, () => {
const { testOptions = {} } = testInfo;
const propNames = Object.keys(componentInfo.props);
Expand All @@ -445,7 +445,7 @@ export const defaultTests: TestObject = {
},

/** Ensures that components have consistent callback arguments (ev, data) */
'consistent-callback-args': (componentInfo, testInfo, tsProgram) => {
'consistent-callback-args': (testInfo, componentInfo, tsProgram) => {
it('has consistent custom callback arguments (consistent-callback-args)', () => {
const { testOptions = {} } = testInfo;

Expand Down Expand Up @@ -501,7 +501,7 @@ export const defaultTests: TestObject = {
},

/** If the primary slot is specified, it receives native props other than 'className' and 'style' */
'primary-slot-gets-native-props': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
'primary-slot-gets-native-props': (testInfo: IsConformantOptions) => {
it(`applies correct native props to the primary and root slots (primary-slot-gets-native-props)`, () => {
try {
const { Component, requiredProps, primarySlot = 'root', renderOptions } = testInfo;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-conformance/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { isConformant } from './isConformant';
export type { ConformanceTest, IsConformantOptions, TestObject, TestOptions } from './types';
export type { BaseConformanceTest, ConformanceTest, IsConformantOptions, TestObject, TestOptions } from './types';
49 changes: 46 additions & 3 deletions packages/react-conformance/src/isConformant.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'fs';

import { IsConformantOptions } from './types';
import { BaseConformanceTest, ConformanceTest, DefaultTestObject, IsConformantOptions } from './types';
import { defaultTests } from './defaultTests';
import { merge } from './utils/merge';
import { createTsProgram } from './utils/createTsProgram';
Expand All @@ -17,6 +17,7 @@ export function isConformant<TProps = {}>(...testInfo: Partial<IsConformantOptio
tsConfig,
// eslint-disable-next-line deprecation/deprecation
tsconfigDir,
disableTypeTests,
} = mergedOptions;

const mergedTsConfig = {
Expand All @@ -29,6 +30,11 @@ export function isConformant<TProps = {}>(...testInfo: Partial<IsConformantOptio
throw new Error(`Path ${componentPath} does not exist`);
}

if (disableTypeTests) {
runNonTypeTests(mergedOptions);
return;
}

const tsProgram = createTsProgram(componentPath, mergedTsConfig);

const components = getComponentDoc(componentPath, tsProgram);
Expand All @@ -39,15 +45,15 @@ export function isConformant<TProps = {}>(...testInfo: Partial<IsConformantOptio

for (const test of Object.keys(defaultTests)) {
if (!disabledTests.includes(test)) {
defaultTests[test](componentInfo, mergedOptions, tsProgram);
defaultTests[test as keyof DefaultTestObject](mergedOptions, componentInfo, tsProgram);
}
}

if (extraTests) {
describe('extraTests', () => {
for (const test of Object.keys(extraTests)) {
if (!disabledTests.includes(test)) {
extraTests[test](componentInfo, mergedOptions, tsProgram);
extraTests[test](mergedOptions, componentInfo, tsProgram);
}
}
});
Expand All @@ -63,3 +69,40 @@ export function isConformant<TProps = {}>(...testInfo: Partial<IsConformantOptio
}
});
}

/**
* Run default and extra tests that don't require TypeScript information
* @param mergedOptions
*/
function runNonTypeTests(mergedOptions: IsConformantOptions) {
const { disabledTests = [], extraTests } = mergedOptions;

for (const test of Object.keys(defaultTests)) {
if (!disabledTests.includes(test)) {
const func = defaultTests[test as keyof DefaultTestObject];
if (isNonTypeTest(func)) {
func(mergedOptions);
}
}
}

if (extraTests) {
describe('extraTests', () => {
for (const test of Object.keys(extraTests)) {
if (!disabledTests.includes(test)) {
const func = extraTests[test];
if (isNonTypeTest(func)) {
func(mergedOptions);
}
}
}
});
}
}

/**
* Verifies that a test function has only one parameter. If so, this test does not require TS info.
*/
function isNonTypeTest<TProps = {}>(func: ConformanceTest<TProps>): func is BaseConformanceTest<TProps> {
return func.length === 1;
}
31 changes: 29 additions & 2 deletions packages/react-conformance/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ export interface IsConformantOptions<TProps = {}> {
* If there are tests that aren't supposed to run on a component, this allows to opt out of any test.
*/
disabledTests?: string[];

/**
* Disable tests that verify the component's prop types.
* These tests require TypeScript information.
* It is recommended to keep these tests enabled, but they can be disabled in a large repo to improve test performance and memory consumption.
*/
disableTypeTests?: boolean;

/**
* Optional flag that means the component is not exported at top level.
* @defaultvalue false
Expand Down Expand Up @@ -104,12 +112,31 @@ export interface IsConformantOptions<TProps = {}> {
tsConfig?: Partial<{ configName: string; configDir: string }>;
}

export type BaseConformanceTest<TProps = {}> = (testInfo: IsConformantOptions<TProps>) => void;
export type ConformanceTest<TProps = {}> = (
componentInfo: ComponentDoc,
testInfo: IsConformantOptions<TProps>,
componentInfo: ComponentDoc,
tsProgram: ts.Program,
) => void;

export interface TestObject<TProps = {}> {
[key: string]: ConformanceTest<TProps>;
[key: string]: BaseConformanceTest<TProps> | ConformanceTest<TProps>;
}

export interface DefaultTestObject<TProps = {}> {
'exports-component': BaseConformanceTest<TProps>;
'component-renders': BaseConformanceTest<TProps>;
'component-has-displayname': BaseConformanceTest<TProps>;
'component-handles-ref': BaseConformanceTest<TProps>;
'component-has-root-ref': BaseConformanceTest<TProps>;
'omits-size-prop': ConformanceTest<TProps>;
'component-handles-classname': BaseConformanceTest<TProps>;
'component-has-static-classnames-object': ConformanceTest<TProps>;
'name-matches-filename': BaseConformanceTest<TProps>;
'exported-top-level': BaseConformanceTest<TProps>;
'has-top-level-file': BaseConformanceTest<TProps>;
'kebab-aria-attributes': ConformanceTest<TProps>;
'consistent-callback-names': ConformanceTest<TProps>;
'consistent-callback-args': ConformanceTest<TProps>;
'primary-slot-gets-native-props': BaseConformanceTest<TProps>;
}

0 comments on commit 8582884

Please sign in to comment.