Skip to content

fix(docs): add overwriteExisting option for flexible README overwrite behaviour #6249

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 6, 2025
Merged

fix(docs): add overwriteExisting option for flexible README overwrite behaviour #6249

merged 4 commits into from
May 6, 2025

Conversation

smorrisods
Copy link
Contributor

@smorrisods smorrisods commented May 2, 2025 •
edited
Loading

The docs-readme output target has long supported writing README files to custom locations using the dir property, helping teams structure their documentation workflows to suit project needs.

As part of evolving those workflows, I've introduced the overwriteExisting property to give developers precise control over how README files are handled. This addition supports common scenarios like always overwriting, only writing when files are missing, or preserving existing content:

  • Always overwrite the full README (true),
  • Only write the full README if no file exists ('if-missing'), or
  • Preserve existing custom content (default false).

These options aim to better support workflows where documentation is closely tied to source code, while also offering flexibility for varied project structures.

What is the current behaviour?

By default, the docs-readme output target preserves any manually-entered content in destination README files, but it does not offer flexible controls for different overwrite behaviours. This made it difficult to manage workflows where documentation is expected to be tightly coupled with source code, while also needing different customisation strategies for various parts of a project.

Link to relevant issues/PRs: #5648 #6248

What is the new behaviour?

This PR introduces documentation for the new overwriteExisting property
on the docs-readme output target.

The documentation includes:

  • Clear description of the overwriteExisting configuration option.
  • Available modes (true, false, and 'if-missing').
  • Example usage in a stencil.config.ts file.
  • Real-world use case highlighting idempotent output and CI workflows.
  • Clarification that the component README beside the component is treated
    as the canonical source of truth for custom content.

Documentation

A documentation update has been made in stenciljs/site#1521.

Does this introduce a breaking change?

  • Yes
  • No

Testing

Other information

See #6248 for a detailed explanation of the use case this PR is covering.

smorrisods added 2 commits May 2, 2025 19:05
@smorrisods
fix(docs): add overwriteExisting option for flexible README overwrite…
34f46f4 
… behaviour

On the `docs-readme` output target, a new `overwriteExisting` option allows developers to
control how existing README files are handled when writing to a custom path.

This adds flexibility to either:

- Always overwrite the full README (`true`),
- Only write the full README if no file exists (`'if-missing'`), or
- Preserve existing custom content (**default** `false`).

Now, `docs-readme` treats the component `readme.md` files as the canonical source for
manually-entered custom content when overwriting READMEs. This ensures that custom content
is preserved unless explicitly overwritten.

For example, when using the `'if-missing'` flag, a project that writes component readmes
into a documentation library folder can maintain consistent, idempotent output across
local builds and CI environments. Custom edits to the destination files will be maintained,
while files without customisation will be fully exported.

Fixes stenciljs#6248
@smorrisods
test(docs-readme): add coverage for overwriteExisting behaviours
ae15cee 
Adds tests covering all `overwriteExisting` modes on the `docs-readme` output target:

- Always overwrite (`true`)
- Overwrite if missing (`'if-missing'`) with and without a destination file
- Never overwrite (`false`)
- Default behaviour (`undefined`), which treats missing values as `false`

Tests confirm that manual content is preserved when appropriate, new files are correctly
generated when missing, and behaviour matches the original fix for stenciljs#5400.

Also validates idempotent output across multiple overwriting scenarios.

Related to the fix for stenciljs#6248.
@smorrisods smorrisods requested a review from a team as a code owner May 2, 2025 23:18
@smorrisods smorrisods mentioned this pull request May 2, 2025
Merged
christian-bromann
christian-bromann approved these changes May 5, 2025
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Great change set!

@smorrisods
Copy link
Contributor Author

Thanks, @christian-bromann! I just noticed a bunch of tests failing, any advice on looking into them?

@christian-bromann
Copy link
Member

You can ignore the failing Component Starter Tests but definitely take a look at the failing docs test which seem to cause changes when running them npm run test.docs-build. Can you identify if these changes are expected? How does the new behavior differ from the old?

@smorrisods
Copy link
Contributor Author

Looking a the docs tests, it looks like it is an issue with Git being dirty after the file that is expected to be overwritten is overwritten. One of the tests replaces one of the readmes with the one that exists within the component, I'm purposefully making sure the file gets overwritten, but this breaks Git. Any thoughts?

image

@smorrisods
Copy link
Contributor Author

One idea that comes to mind. Perhaps I can commit the intended result, and add a step before the docs-readme output target to overwrite it with a sample that will then be overwritten later 🤔. If it doesn't get overwritten back to the expected version then this Git error will show up again and break the tests!

@christian-bromann
Copy link
Member

@smorrisods I think that makes sense

@smorrisods smorrisods force-pushed the fix/docs-readme-control-readme-overwrite-behaviour branch from c352e61 to 686e9c1 Compare May 5, 2025 20:15
@smorrisods
test(docs-readme): revamp overwrite test for consistent test state
d889c08 
Added `copy-readme.js` to copy the supplemental README over the known README result to
ensure the file is in a consistent state before the test begins.

This approach verifies whether the `docs-readme` output target correctly overwrites the
file as expected during the test process.
@smorrisods smorrisods force-pushed the fix/docs-readme-control-readme-overwrite-behaviour branch from 686e9c1 to d889c08 Compare May 5, 2025 20:20
@smorrisods
Copy link
Contributor Author

@christian-bromann, I've made my changes and introduced a script that takes an informational supplemental-readme.md file and copies it into the location of the readme.md file that is expected to be overwritten. The readme.md is copied into place before the build process using the copy-readme.js script.

The intended output is also committed, providing a baseline for what to expect and leveraging the behaviour of the test suite. If the file is successfully overwritten, the checked-in result is restored, and there is no dirty state in Git. However, if the overwrite fails, Git will detect a dirty state, indicating that the operation to overwrite failed, and the test will fail as expected.

@christian-bromann
Copy link
Member

@smorrisods it seems like there is a remaining prettier issue with scripts/test/copy-readme.js, can you run npm run prettier?

@smorrisods
Copy link
Contributor Author

@christian-bromann, good catch! I've prettierified the file and it looks better now. Good catch <3

@christian-bromann christian-bromann added this pull request to the merge queue May 6, 2025
@christian-bromann christian-bromann removed this pull request from the merge queue due to a manual request May 6, 2025
@christian-bromann christian-bromann merged commit fa9a2cc into stenciljs:main May 6, 2025
72 checks passed
@smorrisods smorrisods mentioned this pull request Jun 4, 2025
Merged
smorrisods added a commit to ongov/ontario-design-system that referenced this pull request Jun 9, 2025
@smorrisods
DS-2243: Update Stencil.js and Docusuarus (#63)
6ba7103 
This PR updates Stencil.js and Docusuaus:

### 1. Stencil.js Upgrade
- Upgrades Stencil.js to
[v4.33.1](https://github.com/stenciljs/core/releases/tag/v4.33.1).
- Adds `overwriteExisting: true` to ensure component readme files are
exported to Docusaurus, keeping documentation in sync.
- **Note**: This fixes our component readme documentation generation,
restoring full copies of our component readmes to Docusuaus for
publishing. (Reported via stenciljs/core#6248, fixed via
stenciljs/core#6249 and released in Stencil.js
[v4.31.0](https://github.com/stenciljs/core/releases/tag/v4.31.0))
- Updates all component readmes to reflect changes introduced by the new
Stencil.js version.
- Updates `components.d.ts` to include `@default` values for improved
type safety and documentation clarity.

### 2. Docusaurus Upgrade
- Upgrades Docusaurus to v3.5.0.
- Fixes JSDoc comment formatting in `ontario-hint-text` to resolve
build/runtime issues with Docusaurus v3.
- Cleans up the `prism-react-renderer` import in `docusaurus.config.js`
for compatibility and maintainability.

_With this PR we can now publish production releases again and not
clobber all our documentation!_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christian-bromann christian-bromann christian-bromann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smorrisods @christian-bromann