Skip to content

Commit

Permalink
fix(docs): merge together style docs from multiple CSS files (#5653)
Browse files Browse the repository at this point in the history
This fixes a bug where although multiple stylesheets were being
processed by the `ext-transforms-plugin` we were not properly merging
the style documentation for all of those stylesheets, leading to a race
condition where whichever file was processed last would set the
style docs for the whole component. Not good!

To fix the issue we just need to merge the style docs for the various
stylesheets together (de-duping on the `name` property).

This also adds a test project in `test/docs-readme` which exercises this
functionality.

STENCIL-1271
  • Loading branch information
alicewriteswrongs committed Apr 15, 2024
1 parent 2082368 commit 84e1a14
Show file tree
Hide file tree
Showing 17 changed files with 254 additions and 5 deletions.
1 change: 1 addition & 0 deletions cspell-wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ stenciljs
stnl
stringification
stringified
styleurls
subdir
templating
timespan
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/bundle/ext-transforms-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { hasError, isOutputTargetDistCollection, join, normalizeFsPath, relative } from '@utils';
import { hasError, isOutputTargetDistCollection, join, mergeIntoWith, normalizeFsPath, relative } from '@utils';
import type { Plugin } from 'rollup';

import type * as d from '../../declarations';
Expand Down Expand Up @@ -163,7 +163,8 @@ export const extTransformsPlugin = (

// Set style docs
if (cmp) {
cmp.styleDocs = cssTransformResults.styleDocs;
cmp.styleDocs ||= [];
mergeIntoWith(cmp.styleDocs, cssTransformResults.styleDocs, (docs) => docs.name);
}

// Track dependencies
Expand Down
24 changes: 24 additions & 0 deletions src/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,30 @@ export const unique = <T, K>(array: T[], predicate: (item: T) => K = (i) => i as
});
};

/**
* Merge elements of an array into an existing array, using a predicate to
* determine uniqueness and only adding elements when they are not present in
* the first array.
*
* **Note**: this mutates the target array! This is intentional to avoid
* unnecessary array allocation, but be sure that it's what you want!
*
* @param target the target array, to which new unique items should be added
* @param newItems a list of new items, some (or all!) of which may be added
* @param mergeWith a predicate function which reduces the items in `target`
* and `newItems` to a value which can be equated with `===` for the purposes
* of determining uniqueness
*/
export function mergeIntoWith<T1, T2>(target: T1[], newItems: T1[], mergeWith: (item: T1) => T2) {
for (const item of newItems) {
const maybeItem = target.find((existingItem) => mergeWith(existingItem) === mergeWith(item));
if (!maybeItem) {
// this is a new item that isn't present in `target` yet
target.push(item);
}
}
}

/**
* A utility for building an object from an iterable very similar to
* `Object.fromEntries`
Expand Down
29 changes: 28 additions & 1 deletion src/utils/test/helpers.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dashToPascalCase, isDef, isPromise, toCamelCase, toDashCase } from '../helpers';
import { dashToPascalCase, isDef, isPromise, mergeIntoWith, toCamelCase, toDashCase } from '../helpers';

describe('util helpers', () => {
describe('isPromise', () => {
Expand Down Expand Up @@ -110,4 +110,31 @@ describe('util helpers', () => {
expect(isDef(null)).toBe(false);
});
});

describe('mergeIntoWith', () => {
it('should do nothing if all elements already present', () => {
const target = [1, 2, 3];
mergeIntoWith(target, [1, 2, 3], (x) => x);
expect(target).toEqual([1, 2, 3]);
});

it('should add new items', () => {
const target = [1, 2, 3];
mergeIntoWith(target, [1, 2, 3, 4, 5], (x) => x);
expect(target).toEqual([1, 2, 3, 4, 5]);
});

it('should merge in objects using the predicate', () => {
const target = [{ id: 'foo' }, { id: 'bar' }, { id: 'boz' }, { id: 'baz' }];
mergeIntoWith(target, [{ id: 'foo' }, { id: 'fab' }, { id: 'fib' }], (x) => x.id);
expect(target).toEqual([
{ id: 'foo' },
{ id: 'bar' },
{ id: 'boz' },
{ id: 'baz' },
{ id: 'fab' },
{ id: 'fib' },
]);
});
});
});
13 changes: 13 additions & 0 deletions test/docs-readme/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions test/docs-readme/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "docs-readme-testbed",
"version": "1.0.0",
"description": "A test app for the docs-readme output target",
"files": [
"dist/"
],
"scripts": {
"build": "node ../../bin/stencil build",
"build.dev": "node ../../bin/stencil build --dev",
"start": "node ../../bin/stencil build --dev --watch --serve",
"test": "node ../../bin/stencil test --spec --e2e",
"test.watch": "node ../../bin/stencil test --spec --e2e --watch",
"generate": "node ../../bin/stencil generate"
},
"license": "MIT"
}
13 changes: 13 additions & 0 deletions test/docs-readme/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# docs-readme test app

This directory contains a test application which exercises the `docs-readme`
output target. This provides us with an end-to-end test of this functionality.

## Components

The components in here and what they test!

### `<styleurls-component>`

This tests that the docs from multiple `styleUrls` in the `@Component`
decorator are pulled in to the docs output correctly.
37 changes: 37 additions & 0 deletions test/docs-readme/src/components.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* eslint-disable */
/* tslint:disable */
/**
* This is an autogenerated file created by the Stencil compiler.
* It contains typing information for all components that exist in this project.
*/
import { HTMLStencilElement, JSXBase } from "@stencil/core/internal";
export namespace Components {
interface StyleurlsComponent {
}
}
declare global {
interface HTMLStyleurlsComponentElement extends Components.StyleurlsComponent, HTMLStencilElement {
}
var HTMLStyleurlsComponentElement: {
prototype: HTMLStyleurlsComponentElement;
new (): HTMLStyleurlsComponentElement;
};
interface HTMLElementTagNameMap {
"styleurls-component": HTMLStyleurlsComponentElement;
}
}
declare namespace LocalJSX {
interface StyleurlsComponent {
}
interface IntrinsicElements {
"styleurls-component": StyleurlsComponent;
}
}
export { LocalJSX as JSX };
declare module "@stencil/core" {
export namespace JSX {
interface IntrinsicElements {
"styleurls-component": LocalJSX.StyleurlsComponent & JSXBase.HTMLAttributes<HTMLStyleurlsComponentElement>;
}
}
}
6 changes: 6 additions & 0 deletions test/docs-readme/src/components/styleurls-component/one.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
:host {
/**
* @prop --one: Property One
*/
display: block;
}
18 changes: 18 additions & 0 deletions test/docs-readme/src/components/styleurls-component/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# styleurls-component



<!-- Auto Generated Below -->


## CSS Custom Properties

| Name | Description |
| ------- | ------------ |
| `--one` | Property One |
| `--two` | Property Two |


----------------------------------------------

*Built with [StencilJS](https://stenciljs.com/)*
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Component, h } from '@stencil/core';

@Component({
tag: 'styleurls-component',
shadow: true,
// CSS properties documented in both of these files should
// show up in this component's README
styleUrls: {
one: 'one.scss',
two: 'two.scss',
},
})
export class StyleUrlsComponent {
render() {
return <div>Hello, World! I have multiple style URLs!</div>;
}
}
6 changes: 6 additions & 0 deletions test/docs-readme/src/components/styleurls-component/two.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
:host {
/**
* @prop --two: Property Two
*/
display: block;
}
14 changes: 14 additions & 0 deletions test/docs-readme/src/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<html dir="ltr" lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0" />
<title>Stencil Component Starter</title>

<script type="module" src="/build/json-docs-testbed.esm.js"></script>
<script nomodule src="/build/json-docs-testbed.js"></script>
</head>
<body>
<my-component first="Stencil" last="'Don't call me a framework' JS"></my-component>
</body>
</html>
1 change: 1 addition & 0 deletions test/docs-readme/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './components';
13 changes: 13 additions & 0 deletions test/docs-readme/stencil.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Config } from '@stencil/core';

export const config: Config = {
namespace: 'docs-readme-testbed',
outputTargets: [
{
type: 'docs-readme',
},
{
type: 'dist',
},
],
};
40 changes: 40 additions & 0 deletions test/docs-readme/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"compilerOptions": {
"allowSyntheticDefaultImports": true,
"allowUnreachableCode": false,
"declaration": false,
"experimentalDecorators": true,
"lib": [
"dom",
"es2017"
],
"moduleResolution": "node",
"module": "esnext",
"target": "es2017",
"noUnusedLocals": true,
"noUnusedParameters": true,
"jsx": "react",
"jsxFactory": "h",
"paths": {
"@stencil/core/testing": [
"../../src/testing/index.ts"
],
"@stencil/core": [
"../../internal"
],
"@stencil/core/compiler": [
"../../compiler"
],
"@stencil/core/internal": [
"../../internal"
]
}
},
"include": [
"src"
],
"exclude": [
"node_modules"
]

}
5 changes: 3 additions & 2 deletions test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
"TODO-STENCIL-389": "echo Remove the exit call below",
"analysis": "exit 0 && node ./.scripts/analysis.js",
"analysis.build-and-analyze": "npm run build && npm run analysis",
"build": "npm run build.browser-compile && npm run build.hello-world && npm run build.hello-vdom && npm run build.todo && npm run build.end-to-end && npm run build.ionic && npm run build.docs-json",
"build": "npm run build.browser-compile && npm run build.hello-world && npm run build.hello-vdom && npm run build.todo && npm run build.end-to-end && npm run build.ionic && npm run build.docs-json && npm run build.docs-readme",
"bundlers": "cd ./bundler && npm run start",
"build.browser-compile": "cd ./browser-compile && npm run build",
"build.end-to-end": "cd ./end-to-end && npm ci && npm run build",
"build.hello-world": "cd ./hello-world && npm run build",
"build.hello-vdom": "cd ./hello-vdom && npm run build",
"build.ionic": "cd ./ionic-app && npm ci && npm run build",
"build.todo": "cd ./todo-app && npm ci && npm run build",
"build.docs-json": "cd ./docs-json && npm ci && npm run build"
"build.docs-json": "cd ./docs-json && npm ci && npm run build",
"build.docs-readme": "cd ./docs-readme && npm ci && npm run build"
},
"devDependencies": {
"brotli": "^1.3.2"
Expand Down

0 comments on commit 84e1a14

Please sign in to comment.