Conversation
…ployment Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com> Agent-Logs-Url: https://github.com/rdfjs/wrapper/sessions/0e60820c-d217-4c83-b050-24f294dd5fdd
There was a problem hiding this comment.
Pull request overview
Adds a full MkDocs documentation site for the RDF/JS Wrapper library (guides + API reference), applies Solid-style “Royal Lavender” branding via Material theme overrides, and introduces a GitHub Pages deployment workflow. This modernizes the project docs beyond the existing README and provides a hosted reference for the public API surface.
Changes:
- Added MkDocs Material site configuration, navigation, and authored docs pages under
docs/(guides + API reference). - Added Solid brand styling via
docs/stylesheets/extra.css. - Added GitHub Actions workflow to build/deploy docs to GitHub Pages; ignored the generated
site/output.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| mkdocs.yml | MkDocs Material configuration, nav, and markdown extensions. |
| docs/stylesheets/extra.css | Solid brand color overrides for header/footer/tabs. |
| docs/index.md | Docs homepage with quick example and positioning. |
| docs/getting-started.md | Installation + first mapping walkthrough. |
| docs/guides/term-wrapper.md | TermWrapper conceptual guide + examples. |
| docs/guides/dataset-wrapper.md | DatasetWrapper conceptual guide + examples. |
| docs/guides/nested-objects.md | ObjectMapping-based navigation guide. |
| docs/guides/sets-and-maps.md | Live Set/Map guide + examples. |
| docs/guides/rdf-lists.md | RDF list (ObjectMapping.asList) guide + supported ops. |
| docs/guides/decorators.md | Decorator-based mapping guide + TS configuration note. |
| docs/api/term-wrapper.md | TermWrapper API reference. |
| docs/api/dataset-wrapper.md | DatasetWrapper API reference. |
| docs/api/value-mapping.md | ValueMapping API reference. |
| docs/api/term-mapping.md | TermMapping API reference. |
| docs/api/object-mapping.md | ObjectMapping API reference. |
| docs/api/decorators.md | Decorators API reference. |
| README.md | Restructured README with links to hosted docs and updated examples. |
| .gitignore | Ignores MkDocs output directory (site/). |
| .github/workflows/docs.yml | Builds MkDocs with --strict and deploys to GitHub Pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/api/object-mapping.md
Outdated
| ## ObjectMapping.as | ||
|
|
||
| ```typescript | ||
| ObjectMapping.as<T>(constructor: ITermWrapperConstructor<T>): IValueMapping<T> |
There was a problem hiding this comment.
ObjectMapping.as is described as usable as both a value mapping and a term mapping, but the signature only shows IValueMapping<T>. For clarity in the API reference, consider documenting it as returning an intersection type (e.g. IValueMapping<T> & ITermMapping<T>) or otherwise explicitly noting it is structurally compatible with ITermMapping<T> as well.
| ObjectMapping.as<T>(constructor: ITermWrapperConstructor<T>): IValueMapping<T> | |
| ObjectMapping.as<T>(constructor: ITermWrapperConstructor<T>): IValueMapping<T> & ITermMapping<T> |
docs/guides/sets-and-maps.md
Outdated
| import { TermWrapper, ValueMapping, TermMapping } from "@rdfjs/wrapper" | ||
| import type { ILangString } from "@rdfjs/wrapper" |
There was a problem hiding this comment.
In this map example, TermMapping and ILangString are imported but never used. Removing unused imports will keep the snippet cleaner and reduce copy/paste friction for users (especially in projects with noUnusedLocals).
| import { TermWrapper, ValueMapping, TermMapping } from "@rdfjs/wrapper" | |
| import type { ILangString } from "@rdfjs/wrapper" | |
| import { TermWrapper, ValueMapping } from "@rdfjs/wrapper" |
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| workflow_dispatch: |
There was a problem hiding this comment.
This workflow only runs on push to main, so broken docs can be merged without CI catching it first. Consider adding a pull_request trigger (build-only; no deploy) so mkdocs build --strict runs in PRs before merge.
README.md
Outdated
| import { ObjectMapping } from "@rdfjs/wrapper" | ||
|
|
There was a problem hiding this comment.
This Nested Objects snippet imports only ObjectMapping but uses TermWrapper (and Address) in the example. If the intent is a copy/pasteable snippet, please add the missing imports/definitions; otherwise consider calling out that the example is abbreviated.
| import { ObjectMapping } from "@rdfjs/wrapper" | |
| import { TermWrapper, ObjectMapping } from "@rdfjs/wrapper" | |
| class Address extends TermWrapper { | |
| // Define address-related mappings here, e.g. street, city, etc. | |
| } |
| ```typescript | ||
| @getter("https://schema.org/name", GetterArity.SingularNullable, ValueMapping.literalToString) | ||
| get name(): string | undefined { | ||
| throw new Error() // replaced by decorator |
There was a problem hiding this comment.
The examples below use ValueMapping and TermMapping, but the Import section at the top only shows importing getter, setter, GetterArity, and SetterArity. Consider updating the import snippet (or adding an additional one) so the examples are self-contained/copy-pasteable.
README.md
Outdated
| ```javascript | ||
| import { getter, setter, GetterArity, SetterArity, ValueMapping, TermMapping } from "@rdfjs/wrapper" | ||
|
|
||
| Class usage: | ||
| class Person extends TermWrapper { | ||
| @getter("https://schema.org/name", GetterArity.SingularNullable, ValueMapping.literalToString) | ||
| get name() { throw new Error() } |
There was a problem hiding this comment.
This decorators example is marked as javascript, but it uses TypeScript/TC39 decorators (@getter, @setter). Also, the snippet extends TermWrapper without importing it in this block, so copy/paste won’t run as-is. Consider changing the fence to typescript and including TermWrapper (or explicitly labeling the snippet as partial/pseudocode) plus a short note about the required TS decorator configuration.
docs/api/decorators.md
Outdated
| arity: GetterArity, | ||
| valueMapping: IValueMapping<any>, | ||
| termMapping?: ITermMapping<any>, | ||
| ): ClassGetterDecoratorContext => ClassGetterDecoratorContext |
There was a problem hiding this comment.
The documented getter(...) signature/return type here doesn’t match the actual implementation (a decorator factory that returns a function (target, context) => replacementGetter). As written, ClassGetterDecoratorContext => ClassGetterDecoratorContext is not a correct type shape and may confuse readers. Please update the signature to reflect that getter(...) returns a class getter decorator function.
| ): ClassGetterDecoratorContext => ClassGetterDecoratorContext | |
| ): (target: unknown, context: ClassGetterDecoratorContext) => () => unknown |
docs/api/decorators.md
Outdated
| predicate: string, | ||
| arity: SetterArity, | ||
| termMapping: ITermMapping<any>, | ||
| ): ClassSetterDecoratorContext => ClassSetterDecoratorContext |
There was a problem hiding this comment.
Similarly, the documented setter(...) signature/return type doesn’t match the implementation (a decorator factory returning (target, context) => replacementSetter). Updating the type shape here will make the API reference accurate for TypeScript users.
| ): ClassSetterDecoratorContext => ClassSetterDecoratorContext | |
| ): (target: any, context: ClassSetterDecoratorContext) => (value: any) => void |
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com> Agent-Logs-Url: https://github.com/rdfjs/wrapper/sessions/e3ef5d26-0106-4349-ad27-8c32daab5868
Applied all review feedback in commit
|
There was a problem hiding this comment.
This one is interesting @langsamu -- I thought it was an hallucination at first because I've not encountered decorators used this way in typescript, but it turns out this syntax is indeed valid as of TypeScript 5
|
@copilot In order to make sure that the examples are correct, and that the documentation remains up-to-date with breaking changes in the library; during CI, please extract all of the examples from the documentation, execute and test them in order to ensure:
Similarly to the existing tests, please also produce a report of the coverage provided by the documentation files; to ensure that we sufficiently cover the documented API's in the documentation. |
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com> Agent-Logs-Url: https://github.com/rdfjs/wrapper/sessions/459be909-f8db-4c93-a770-c6863527f1f8
Done in commit
The docs tests are automatically included in
Also fixed a doc bug found during testing: the "Full Decorated Class Example" in |
|
@copilot there should not be duplication of code between the new test files and the docs in order to avoid them getting out of sync. Please write a script which extracts the examples from the test files and adds them to the docs. |
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com> Agent-Logs-Url: https://github.com/rdfjs/wrapper/sessions/ecc94eca-c42b-4bdf-95c6-d0ba131dbbed
Done in commit
Each file contains only the class definition(s) with their
Docs — each Complete Example code block is now split into a synced class-definition block (marked with |
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
|
@copilot please add CI to check that the doc examples are in sync |
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com> Agent-Logs-Url: https://github.com/rdfjs/wrapper/sessions/dab6807e-b68a-4917-80e1-afa0ee41a930
Done in commit Running the check also revealed two stale imports in the docs (an unused |
|
@langsamu I am happy with this PR, could you please take a quick glance and then merge. |
The package had no formal documentation beyond a minimal README. This adds a full MkDocs site covering all public API surface, deployed to GitHub Pages, styled with the Solid Project brand colour (Royal Lavender
#7C4DFF). Documentation examples are executed as part of CI with coverage reporting, and a sync script keeps example code in the docs and tests in lock-step.README
TermWrapperimport andAddressstub)typescriptfence withTermWrapperimport and proper typesMkDocs site (
docs/)TermWrapper,DatasetWrapper, nested objects, sets/maps, RDF lists, decorators; each with runnable examplesTermWrapper,DatasetWrapper,ValueMapping,TermMapping,ObjectMapping, decorators; full method signatures and parameter tablesObjectMapping.asreturn type corrected toIValueMapping<T> & ITermMapping<T>;getter()/setter()return types corrected to accurate decorator factory shapes; import sections updated to include all referenced symbolsdocs/api/decorators.mdwith a corrected Full Decorated Class Example usingFriend/PersonDecoratedsplitStyling (
docs/stylesheets/extra.css)Overrides Material theme CSS variables to apply Royal Lavender (
#7C4DFF) to the masthead, footer, and nav tabs per the Solid style guide.CI (
mkdocs.yml+.github/workflows/docs.yml+.github/workflows/ci.yml)mkdocs-materialbuild gated with--strict; deploys to GitHub Pages on push tomainviaactions/deploy-pages. Apull_requesttrigger runs the build-only step on PRs to catch breakage before merge.npm run check-doc-examplesis added as a step in thetestjob of.github/workflows/ci.yml, running on every push across all OS matrix targets. This ensures doc code blocks never drift from the canonical example files intest/docs/examples/.site/added to.gitignore.Documentation example tests (
test/docs/)Example code is stored once in
test/docs/examples/(the single source of truth) and shared between the test suite and the documentation via a sync script — eliminating duplication and keeping both in lock-step.test/docs/examples/— 10 TypeScript files (single source of truth)Each file contains only the class definition(s) with their
@rdfjs/wrapperimports. These are what tests import and what docs display:term-wrapper.tsBooknested-objects.tsAddress,Personsets-and-maps-article.tsArticlesets-and-maps-person.tsPerson(with friendsSet)sets-and-maps-map.tsResourcerdf-lists.tsPlaylist,EXdataset-wrapper.tsPerson,Peopledecorators-article.tsTag,Articledecorators-equivalence.tsArticleManual,ArticleDecorateddecorators-person.tsFriend,PersonDecoratedtest/docs/*.test.ts— 6 test files import classes from the example files and add assertions. No class body duplication.scripts/sync-doc-examples.mjs— reads each example file and updates the matching<!-- example: STEM -->…<!-- /example -->slot in the doc markdown:npm run sync-doc-examples— updates docs in placenpm run check-doc-examples— exits 1 if any doc is out of sync (enforced in CI via.github/workflows/ci.yml)test/tsconfig.jsonupdated to compiletest/docs/(includingtest/docs/examples/). A dedicatednpm run test:docsscript runs only the documentation tests with--experimental-test-coverageto produce a docs-specific coverage report.Coverage impact:
npm run test:docs): lines 76.30%, branch 91.02%, functions 61.73%Original prompt
Created from VS Code.
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.