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

Add lit-labs/context docs #1100

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add lit-labs/context docs #1100

wants to merge 6 commits into from

Conversation

AndrewJakubowicz
Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz commented May 5, 2023

Add (correctly this time), the lit-labs/context documentation.

Note - there is still a strange issue with some of the complex types. This issue is what made me notice #1099

Depends on #1093

@github-actions
Copy link

github-actions bot commented May 5, 2023

A live preview of this PR will be available at the URL(s) below.
The latest URL will be appended to this comment on each push.
Each build takes ~5-10 minutes, and will 404 until finished.

https://pr1100-51a9bb2---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1100-fa63b3b---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1100-495af6e---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1100-ca5321d---lit-dev-5ftespv5na-uc.a.run.app/

@justinfagnani
Copy link
Contributor

This is looking relatively good. We need a lot more jsdoc in the package, and it'd be really nice to be able to order things so that createContext, consume and provide should up first.

Base automatically changed from update-docs-typedoc-2 to main May 5, 2023 18:06
/** @lit-labs/context symbol ordering */
'createContext',
'consume',
'provide',
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add ContextConsumer, ContextProvider, and ContextRoot here for now.

tsConfigPath: pathlib.join(contextDir, 'tsconfig.json'),
entrypointModules: [
pathlib.join(contextSrcDir, 'index.ts'),
pathlib.join(contextSrcDir, 'lib/context-request-event.ts'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these other files as entrypoint modules? Since we want everything importing from index, can typedoc not just find them in the module graph?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree - I have to investigate this now that I have stabilized updating all the underlying stuff & fixed many other confounding issues.

Previously when I only used the entrypoint we only got a small cross section of APIs generated. See https://pr1092-ec9e4af---lit-dev-5ftespv5na-uc.a.run.app/docs/api/context/ as an example.

But many things have been fixed since then & I understand what is going on better now. Will try this again to clean it up.
Great comment!

@@ -225,6 +264,14 @@ export const lit2Config: ApiDocsConfig = {
// fine in practice, but when we add e.g. @lit/localize we'll need to be
// smarter here.
let [_, pkg, pathMinusExtension] = match;

if (pkg === 'labs/context') {
console.log(pkg, pathMinusExtension);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

const allPages: Pages = [];
const allSymbols: SymbolMap = {};
for (const pkg of config.packages) {
const app = new typedoc.Application();
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point I thought I read some docs that typedoc had options for a monorepo, so you didn't need to make a new app for each package. I can't remember it though. This might matter for getting crosslinks between packages. Right now ReactiveElement say isn't a link in the context docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - I need to investigate this further. I feel like I read that somewhere as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants