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

feat: separating non-react specific utils and type definitions into it's own package. #2751

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
69 changes: 37 additions & 32 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,28 @@

## Table Of Contents

- [Intro](#intro)
- [Install Elements](#install-elements)
- [Develop Elements](#develop-elements)
- [Testing](#testing)
- [Guiding principles](#guiding-principles)
- [Unit tests](#unit-tests)
- [Run unit-tests](#run-unit-tests)
- [Framework Integration](#framework-integration)
- [Run tests as the CI would](#run-tests-as-the-ci-would)
- [Run the tests manually](#run-the-tests-manually)
- [Edit the tests](#edit-the-tests)
- [Inspect test results](#inspect-test-results)
- [Yalc into platform-internal](#yalc-into-platform-internal)
- [Release Elements](#release-elements)
- [Versioning Guidelines](#versioning-guidelines)
- [Merge into main](#merge-into-main)

- [Contributing to Stoplight Elements](#contributing-to-stoplight-elements)
- [Table Of Contents](#table-of-contents)
- [Intro](#intro)
- [Install Elements](#install-elements)
- [Develop Elements](#develop-elements)
- [Testing](#testing)
- [Guiding Principles](#guiding-principles)
- [Unit tests](#unit-tests)
- [Run Unit Tests](#run-unit-tests)
- [Framework Integration](#framework-integration)
- [Run Tests as the CI Would](#run-tests-as-the-ci-would)
- [Run the Tests Manually](#run-the-tests-manually)
- [Edit the Tests](#edit-the-tests)
- [Inspect Test Results](#inspect-test-results)
- [Yalc into platform-internal](#yalc-into-platform-internal)
- [Release Elements](#release-elements)
- [Versioning Guidelines](#versioning-guidelines)
- [Major versions](#major-versions)
- [Minor versions](#minor-versions)
- [Patches](#patches)
- [Merge into Main](#merge-into-main)

## Intro

Elements is an open-source project, and Stoplight loves contributions. If you're familiar with TypeScript and Jest then dive right in, see how far you can get, and post in [Discussions](https://github.com/stoplightio/elements/discussions) or start a draft PR if you get stuck.
Expand All @@ -41,39 +46,39 @@ To validate that the installation was successful, move into the demo folder by r

## Develop Elements

Elements is split into three packages. Two of them, `elements` and `elements-dev-portal`, are user-facing. The third, `elements-core`, is an implementation detail created to share code and components between `elements` and `elements-dev-portal`.
Elements is split into four packages. Two of them, `elements`, and `elements-dev-portal`, are user-facing. The thirts, `elements-utils` holds implementations and definitions of shared non-react specific code, the fourth, `elements-core`, is an implementation detail created to share code and components between `elements` and `elements-dev-portal`.

Most of the code is in `elements-core`; `elements` and `elements-dev-portal` only have code that's highly specific to those projects.

Most often, you'll develop Elements (in all the packages) using [storybooks](https://storybook.js.org/).

Each package has its own storybook. To run a storybook for a specific package, in the main directory run, for example, `yarn elements-core storybook`. This starts a storybook for the `elements-core` package.

Now you can develop the code and test your changes in the storybook.
Now you can develop the code and test your changes in the storybook.

For your convenience, all the packages are linked. For example, if you run the `elements` storybook, but make changes in the `elements-core` codebase, those changes *will be* visible instantly in your storybook.

## Testing

### Guiding Principles

The aim should never be to write tests for the sake of writing tests.
The aim should never be to write tests for the sake of writing tests.

For Stoplight, the goal of testing is **to give developers confidence** when making changes to the codebase when they're adding new features, cleaning up tech debt, or fixing bugs.

Well-written tests also **save time** when authoring or reviewing PRs as you don't have to run through hundreds of test cases manually to verify everything still works as intended. On the other hand, **badly written tests** that depend on the implementation need to be changed any time the implementation changes, **causing frustration and unnecessary work**, while barely adding any value.

To achieve high-quality tests, **follow these principles**:
- Always test the **behavior** of a component, not the implementation.
- Tests that use *Jest snapshots* almost always violate this. (Except maybe when you are testing an AST parser, a linter, or similar.)
- Tests that use *Jest snapshots* almost always violate this. (Except maybe when you are testing an AST parser, a linter, or similar.)

**Instead,** extract the real business requirement from what would be the snapshot and assert against that.
- Tests that `find` child components and assert against props being passed may be incorrect.

- Tests that `find` child components and assert against props being passed may be incorrect.
Use the **recommended selectors** (see point below) and **[`jest-dom` assertions](https://github.com/testing-library/jest-dom)** to enforce constraints that matter to the user.
- Searching for DOM elements using tag name, CSS class, or hierarchy (`parentElement`, etc.) is an anti-pattern.

**Instead,** use **`findByRole` or other queries from [TL's query hierarchy](https://testing-library.com/docs/queries/about#priority)**.
**Instead,** use **`findByRole` or other queries from [TL's query hierarchy](https://testing-library.com/docs/queries/about#priority)**.
Feel free to add accessibility attributes where missing. With a bit of practice, you'll see that almost everything can be covered with `*byRole`.
- The goal for your test suite is **to cover** as much of the **business requirements** (for example, in the issue description) as practical.
- An ideal test suite only requires a change **if business requirements change**.
Expand All @@ -95,11 +100,11 @@ Unit testing stack:
- [JSDOM](https://github.com/jsdom/jsdom)
- [React Testing Library](https://github.com/testing-library/react-testing-library/)
- [Jest-DOM](https://github.com/testing-library/jest-dom)
- \* You can find some legacy code utilizing a different stack (Enzyme). When changing those tests, use your judgment
- \* You can find some legacy code utilizing a different stack (Enzyme). When changing those tests, use your judgment
to decide between amending the old unit test or rewriting it using the new stack. Mixing testing libraries in a single test file is fine.

Unit tests are currently located in a directory `__tests__` close to the component being tested, but in the future, tests will be located right next to the components under test, with a `.spec.ts` extension.

#### Run Unit Tests

Assuming you work on the `elements` package, you can run `jest` on it using the shorthand
Expand Down Expand Up @@ -144,7 +149,7 @@ That being said, if you for some reason want to run them by hand, here's how to

```shell
# Make sure top-level dependencies are up-to-date
yarn
yarn
# Build Elements itself
yarn build
# Copy the contents of the predefined example application and make it use the local build
Expand All @@ -164,7 +169,7 @@ The first three steps are the same, but this time, instead of running the tests

```shell
# Make sure top-level dependencies are up-to-date
yarn
yarn
# Build Elements itself
yarn build
# Copy the contents of the predefined example application and make it use the local build
Expand All @@ -190,7 +195,7 @@ Fixtures, *Cypress* plugins, and support files can also be found in the relevant

#### Inspect Test Results

Test results can be found under the `cypress/results` directory.
Test results can be found under the `cypress/results` directory.
*Cypress* records videos of every test suite run and takes screenshots for every failure. In addition, a machine-readable and human-readable `output.xml` is generated that contains a summary of the results.

When running in *CircleCI*, the host interprets `output.xml` and displays it visually on the dashboard:
Expand Down Expand Up @@ -237,7 +242,7 @@ If you think a major version bump is required in *any* `elements` package, pleas

Minor versions in `elements` and `elements-dev-portal` are for introducing new features. If *any* change that's being released introduces a new feature / somehow extends the functionality, bump the minor.

In the case of `elements-core` (and in contrast with two other packages), minors are allowed to have (within reason) some breaking changes. That's because it's an internal package that Stoplight controls.
In the case of `elements-core` (and in contrast with two other packages), minors are allowed to have (within reason) some breaking changes. That's because it's an internal package that Stoplight controls.

If you need to make a breaking change in `elements-core`, make sure to bump minor *and* make sure that the new versions of `elements` and `elements-dev-portal` are using this new version and are compatible with it. Remember also that `elements` is used in internal Stoplight platform code, so make sure that the new version also works correctly there.

Expand Down
9 changes: 6 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,23 @@
"@stoplight/react-error-boundary": "3.0.0"
},
"scripts": {
"postinstall": "yarn workspace @stoplight/elements-utils build",
"demo": "yarn workspace @stoplight/elements-demo",
"elements": "yarn workspace @stoplight/elements",
"elements-core": "yarn workspace @stoplight/elements-core",
"elements-dev-portal": "yarn workspace @stoplight/elements-dev-portal",
"build": "yarn workspace @stoplight/elements-core build && yarn workspace @stoplight/elements build && yarn workspace @stoplight/elements-dev-portal build",
"elements-utils": "yarn workspace @stoplight/elements-utils",
"build": "yarn workspace @stoplight/elements-utils build && yarn workspace @stoplight/elements-core build && yarn workspace @stoplight/elements build && yarn workspace @stoplight/elements-dev-portal build",
"build.docs": "yarn workspace @stoplight/elements build.docs",
"postbuild": "yarn workspace @stoplight/elements test.packaging && yarn workspace @stoplight/elements-core test.packaging && yarn workspace @stoplight/elements-dev-portal test.packaging",
"lint": "eslint '{packages,examples}/*/src/**/*.{ts,tsx}'",
"lint1": "eslint 'packages/elements-utils/src/**/*.{ts,tsx}'",
"version": "lerna version --no-push",
"release": "lerna publish from-package --yes --registry https://registry.npmjs.org --loglevel silly",
"release.docs": "yarn workspace @stoplight/elements release.docs",
"type-check": "yarn workspaces run type-check",
"test": "yarn workspace @stoplight/elements test && yarn workspace @stoplight/elements-core test && yarn workspace @stoplight/elements-dev-portal test --passWithNoTests",
"test.prod": "yarn workspace @stoplight/elements test.prod && yarn workspace @stoplight/elements-core test.prod && yarn workspace @stoplight/elements-dev-portal test.prod --passWithNoTests",
"test": "yarn workspace @stoplight/elements test && yarn workspace @stoplight/elements-core test && yarn workspace @stoplight/elements-dev-portal test --passWithNoTests && yarn workspace @stoplight/elements-utils test --passWithNoTests",
"test.prod": "yarn workspace @stoplight/elements test.prod && yarn workspace @stoplight/elements-core test.prod && yarn workspace @stoplight/elements-dev-portal test.prod --passWithNoTests && yarn workspace @stoplight/elements-utils test.prod --passWithNoTests",
"prepublishOnly": "yarn build",
"/// examples ///": "",
"copy:angular": "mkdir examples-dev ; cp -a -v ./examples/angular ./examples-dev ; sh ./use-local-elements.sh angular",
Expand Down
1 change: 1 addition & 0 deletions packages/elements-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
]
},
"dependencies": {
"@stoplight/elements-utils": "^0.0.1",
"@stoplight/http-spec": "^7.1.0",
"@stoplight/json": "^3.21.0",
"@stoplight/json-schema-ref-parser": "^9.2.7",
Expand Down
62 changes: 11 additions & 51 deletions packages/elements-core/src/components/TableOfContents/types.ts
Original file line number Diff line number Diff line change
@@ -1,51 +1,11 @@
export type TableOfContentsProps = {
tree: TableOfContentsItem[];
activeId: string;
Link: CustomLinkComponent;
maxDepthOpenByDefault?: number;
externalScrollbar?: boolean;
isInResponsiveMode?: boolean;
onLinkClick?(): void;
};

export type CustomLinkComponent = React.ComponentType<{
to: string;
className?: string;
children: React.ReactNode;
}>;

export type TableOfContentsItem = TableOfContentsDivider | TableOfContentsGroupItem;

export type TableOfContentsDivider = {
title: string;
};

export type TableOfContentsGroupItem =
| TableOfContentsGroup
| TableOfContentsNodeGroup
| TableOfContentsNode
| TableOfContentsExternalLink;

export type TableOfContentsGroup = {
title: string;
items: TableOfContentsGroupItem[];
itemsType?: 'article' | 'http_operation' | 'http_webhook' | 'model';
};

export type TableOfContentsExternalLink = {
title: string;
url: string;
};

export type TableOfContentsNode<
T = 'http_service' | 'http_operation' | 'http_webhook' | 'model' | 'article' | 'overview',
> = {
id: string;
slug: string;
title: string;
type: T;
meta: string;
version?: string;
};

export type TableOfContentsNodeGroup = TableOfContentsNode<'http_service'> & TableOfContentsGroup;
export type {
CustomLinkComponent,
TableOfContentsDivider,
TableOfContentsExternalLink,
TableOfContentsGroup,
TableOfContentsGroupItem,
TableOfContentsItem,
TableOfContentsNode,
TableOfContentsNodeGroup,
TableOfContentsProps,
} from '@stoplight/elements-utils';
14 changes: 7 additions & 7 deletions packages/elements-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@ export { ReactRouterMarkdownLink } from './components/MarkdownViewer/CustomCompo
export { NonIdealState } from './components/NonIdealState';
export { PoweredByLink } from './components/PoweredByLink';
export { TableOfContents } from './components/TableOfContents';
export {
CustomLinkComponent,
TableOfContentsGroup,
TableOfContentsItem,
TableOfContentsNode,
TableOfContentsNodeGroup,
} from './components/TableOfContents/types';
export { findFirstNode } from './components/TableOfContents/utils';
export { TryIt, TryItProps, TryItWithRequestSamples, TryItWithRequestSamplesProps } from './components/TryIt';
export { HttpMethodColors, NodeTypeColors, NodeTypeIconDefs, NodeTypePrettyName } from './constants';
Expand All @@ -44,3 +37,10 @@ export { ReferenceResolver } from './utils/ref-resolving/ReferenceResolver';
export { createResolvedObject } from './utils/ref-resolving/resolvedObject';
export { slugify } from './utils/string';
export { createElementClass } from './web-components/createElementClass';
export type {
CustomLinkComponent,
TableOfContentsGroup,
TableOfContentsItem,
TableOfContentsNode,
TableOfContentsNodeGroup,
} from '@stoplight/elements-utils';
56 changes: 8 additions & 48 deletions packages/elements-core/src/utils/guards.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,8 @@
import type { IMarkdownViewerProps } from '@stoplight/markdown-viewer';
import { isArray } from '@stoplight/mosaic';
import { IHttpOperation, IHttpService, IHttpWebhookOperation, INode } from '@stoplight/types';
import { JSONSchema7 } from 'json-schema';
import { isObject, isPlainObject } from 'lodash';

export function isSMDASTRoot(maybeAst: unknown): maybeAst is IMarkdownViewerProps['markdown'] {
return (
isObject(maybeAst) &&
maybeAst['type' as keyof IMarkdownViewerProps['markdown']] === 'root' &&
isArray(maybeAst['children' as keyof IMarkdownViewerProps['markdown']])
);
}

export function isJSONSchema(maybeSchema: unknown): maybeSchema is JSONSchema7 {
// the trick is, JSONSchema doesn't define any required properties, so technically even an empty object is a valid JSONSchema
return isPlainObject(maybeSchema);
}

function isStoplightNode(maybeNode: unknown): maybeNode is INode {
return isObject(maybeNode) && 'id' in maybeNode;
}

export function isHttpService(maybeHttpService: unknown): maybeHttpService is IHttpService {
return isStoplightNode(maybeHttpService) && 'name' in maybeHttpService && 'version' in maybeHttpService;
}

export function isHttpOperation(maybeHttpOperation: unknown): maybeHttpOperation is IHttpOperation {
return isStoplightNode(maybeHttpOperation) && 'method' in maybeHttpOperation && 'path' in maybeHttpOperation;
}

export function isHttpWebhookOperation(
maybeHttpWebhookOperation: unknown,
): maybeHttpWebhookOperation is IHttpWebhookOperation {
return (
isStoplightNode(maybeHttpWebhookOperation) &&
'method' in maybeHttpWebhookOperation &&
'name' in maybeHttpWebhookOperation
);
}

const properUrl = new RegExp(
/((([A-Za-z]{3,9}:(?:\/\/)?)(?:[\-;:&=\+\$,\w]+@)?[A-Za-z0-9\.\-]+|(?:www\.|[\-;:&=\+\$,\w]+@)[A-Za-z0-9\.\-]+)((?:\/[\+~%\/\.\w\-_]*)?\??(?:[\-\+=&;%@\.\w_]*)#?(?:[\.\!\/\\\w]*))?)/,
);

export function isProperUrl(url: string) {
return properUrl.test(url);
}
export {
isHttpOperation,
isHttpService,
isHttpWebhookOperation,
isJSONSchema,
isProperUrl,
isSMDASTRoot,
} from '@stoplight/elements-utils';
Loading