rewrite components doc generator #58

just-boris · 2025-04-03T17:38:39Z

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2025-04-03T17:40:04Z

Codecov Report

Attention: Patch coverage is 95.70312% with 11 lines in your changes missing coverage. Please review.

Project coverage is 90.02%. Comparing base (5bc2709) to head (0029258).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/bootstrap/typescript.ts	89.28%	3 Missing ⚠️
src/components/extractor.ts	96.70%	3 Missing ⚠️
src/components/type-utils.ts	90.32%	3 Missing ⚠️
src/components/object-definition.ts	95.55%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (5bc2709) and HEAD (0029258). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (5bc2709) HEAD (0029258)

2 1

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
- Coverage   95.41%   90.02%   -5.40%     
==========================================
  Files          15       16       +1     
  Lines         436      461      +25     
  Branches      149      129      -20     
==========================================
- Hits          416      415       -1     
- Misses         20       44      +24     
- Partials        0        2       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

just-boris · 2025-04-08T10:04:03Z

fixtures/components/complex-types/column-layout/index.tsx

+  export type Columns = 1 | 2 | 3 | 4;
+  export type Widths = 25 | '50%' | 100 | '33%';


Added new test case on number-only and on mixed string+number union types.

We do not have a real use-case yet, but let's make sure it works anyway

just-boris · 2025-04-08T10:04:44Z

fixtures/components/complex-types/table/index.tsx

 import * as React from 'react';
 import { TableProps } from './interfaces';

+export { TableProps };


The new implementation is more strict, it checks that the interface is actually exported in the index file (which was our requirement all the time)

just-boris · 2025-04-08T10:06:09Z

fixtures/components/third-party-import-types/button/interfaces.ts

 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import { IconProps } from '../node_modules_mock/icon';
+import { IconProps } from 'icon';


Updated this test to more accurately represent node_modules resolution. node_modules_mock is a normal project folder, typescript has no special logic there.

just-boris · 2025-04-08T10:07:19Z

src/bootstrap/typescript.ts

+import ts from 'typescript';
+import pathe from 'pathe';
+
+function printDiagnostics(diagnostics: readonly ts.Diagnostic[]): void {


Official docs for API used in this file: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API

just-boris · 2025-04-08T10:09:55Z

src/components/index.ts

+  // deprecated, now unused
+  additionalInputFilePaths?: Array<string>,


Was added some time ago: #43

In the new implementation, the workaround is not needed. Imports from node_modules just work, as the updated test-case shows

just-boris · 2025-04-08T10:15:28Z

src/components/extractor.ts

+  // disabled until migration is complete
+  // if (unknownExports.length > 0) {
+  // throw new Error(`Unexpected exports in ${componentName}: ${unknownExports.join(', ')}`);
+  // }


The idea is to fail the build if a button/index.tsx file exports anything other than ButtonProps and Button (as a default export).

But this will require additional work in downstream repositories. I will enable it after migration is completed.

just-boris · 2025-04-08T10:20:04Z

src/components/type-utils.ts

+function stripUndefined(typeString: string) {
+  return typeString.replace(/\| undefined$/, '').trim();
+}


Tried multiple options here

type.getNonNullableType() allows to remove both undefined and null, but we want nulls still to be printed. Sadly, type.getNonUndefinedType() does not exist

Typedoc created a custom function for this use-case. But it mutates the type in-place which is concerning. Not an option.

That's how I landed on "fixing" the printed value as the safest and most future proof option which does not depend on any internals

just-boris · 2025-04-08T10:21:16Z

test/components/analytics-tag.test.ts

      {
        name: 'analyticsMetadata',
        analyticsTag: 'View details in Analytics tab',
-        defaultValue: undefined,


When comparing values, Jest ignores undefined anyway. Removed them in a bunch of places to keep the assertions shorter, more readable

just-boris · 2025-04-08T10:22:36Z

test/components/portals.test.ts


 test('should detect component which uses createPortals', () => {
  expect(component.name).toEqual('SimplePortal');
-  expect(component.description).toEqual('Component-level description');


Deprecated this feature

just-boris · 2025-04-08T10:22:54Z

test/components/test-helpers.ts

 import { bootstrapProject } from '../../src/bootstrap';
 import { TestUtilsDoc } from '../../src/test-utils/interfaces';

-// TODO: Move this file into common location, improve naming


this TODO is not going to happen

just-boris · 2025-04-08T10:32:20Z

src/components/index.ts

+      // istanbul ignore next
+      if (!moduleSymbol) {
+        throw new Error(`Unable to resolve module: ${sourceFile.fileName}`);
+      }


Typescript says the value could be undefined, but there is no real use-case for it.

Decided to throw an error just in case, but this code is not testable.

abdhalees · 2025-04-08T15:48:52Z

src/components/component-definition.ts

+        isDefault: region.name === 'children',
+        visualRefreshTag: getCommentTag(region, 'visualrefresh'),
+        deprecatedTag: getCommentTag(region, 'deprecated'),
+        i18nTag: castI18nTag(getCommentTag(region, 'i18n')),


do we have any cases where there is i18n support for regions?

Not sure about anywhere in the components, but in this repo we have a unit test for it

documenter/fixtures/components/i18n-tag/example/index.tsx

Lines 17 to 22 in 5bc2709

/**

* Main content

* @displayname content

* @i18n

*/

children?: React.ReactNode;

So I decided I keep it

just-boris force-pushed the components-without-typedoc branch 4 times, most recently from 272bb04 to 8eacf65 Compare April 3, 2025 18:06

rewrite components doc generator

84993fb

just-boris force-pushed the components-without-typedoc branch from 8eacf65 to 84993fb Compare April 3, 2025 18:30

restore string unions behavior

006e127

just-boris requested a review from georgylobko April 8, 2025 09:39

just-boris marked this pull request as ready for review April 8, 2025 09:39

just-boris requested a review from a team as a code owner April 8, 2025 09:39

just-boris removed the request for review from a team April 8, 2025 09:39

fix ref issues

b382d76

just-boris force-pushed the components-without-typedoc branch from c08a6b9 to b382d76 Compare April 8, 2025 10:13

just-boris commented Apr 8, 2025

View reviewed changes

just-boris requested review from a team and abdhalees and removed request for a team and georgylobko April 8, 2025 15:09

abdhalees approved these changes Apr 8, 2025

View reviewed changes

add more tests

0029258

just-boris force-pushed the components-without-typedoc branch from e69f8ee to 0029258 Compare April 8, 2025 17:26

just-boris enabled auto-merge April 8, 2025 17:35

just-boris added this pull request to the merge queue Apr 8, 2025

Merged via the queue into main with commit 14530c1 Apr 8, 2025
35 checks passed

just-boris deleted the components-without-typedoc branch April 8, 2025 18:04

		export type Columns = 1 \| 2 \| 3 \| 4;
		export type Widths = 25 \| '50%' \| 100 \| '33%';

		// deprecated, now unused
		additionalInputFilePaths?: Array<string>,

	/**
	* Main content
	* @displayname content
	* @i18n
	*/
	children?: React.ReactNode;

rewrite components doc generator #58

rewrite components doc generator #58

Uh oh!

Conversation

just-boris commented Apr 3, 2025

Uh oh!

codecov bot commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Apr 3, 2025 •

edited

Loading