Skip to content

Update to typedoc 0.28 and use entryPointStrategy: packages #638

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

Merged
merged 3 commits into from
Mar 25, 2025

Conversation

eemeli
Copy link
Member

@eemeli eemeli commented Mar 25, 2025

Builds the API documentation together for all packages, adding an index file and linkifying the packages.

Redirects are included from the old docs locations to the new paths.

Documentation tags and layouts are fixed and terms linkified where appropriate.

In @fluent/syntax, the FluentParser internal methods are left out of the public docs, as they are not usable without a FluentParserStream instance, which is not available outside the package.

The TS path mappings needed to be moved to the root; this should have no effect on the TS build.

@eemeli eemeli requested a review from flodolo March 25, 2025 11:41
Copy link

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

I get this trying to run npm install and npm run doc? It's a fresh clone with this branch checked out

fluent-react/src/localization.ts:1:46 - error TS2307: Cannot find module '@fluent/bundle' or its corresponding type declarations.

1 import { FluentBundle, FluentVariable } from "@fluent/bundle";
                                               ~~~~~~~~~~~~~~~~

fluent-react/src/localization.ts:2:31 - error TS2307: Cannot find module '@fluent/sequence' or its corresponding type declarations.

2 import { mapBundleSync } from "@fluent/sequence";
                                ~~~~~~~~~~~~~~~~~~

fluent-react/src/localized.ts:8:32 - error TS2307: Cannot find module '@fluent/bundle' or its corresponding type declarations.

8 import { FluentVariable } from "@fluent/bundle";
                                 ~~~~~~~~~~~~~~~~

fluent-react/src/with_localization.ts:3:32 - error TS2307: Cannot find module '@fluent/bundle' or its corresponding type declarations.

3 import { FluentVariable } from "@fluent/bundle";
                                 ~~~~~~~~~~~~~~~~

[info] Converting project at ./fluent-sequence
[info] Converting project at ./fluent-syntax
[error] Failed to convert one or more packages, result will not be merged together
[error] Found 5 errors and 0 warnings

This is on macOS, node v20.17.0, npm 10.9.0

Copy link

Choose a reason for hiding this comment

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

This document say npm run dist is equivalent to a bunch of commands, but npm run docs is not really in that list?

@flodolo
Copy link

flodolo commented Mar 25, 2025

Correction: it looks like you need to run npm run dist (or probably npm run build) once to stop getting that error. Still, the note on npm run dist including docs generation remains valid.

Copy link

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

Just to confirm, I see

"postdist": "npm run lint && npm run test && npm run docs",

But the html folder is not created by npm run dist.

Besides that, the docs look good. I wish we could avoid the weird <br> in the comments, but that would require disabling trimming of trailing whitespaces, and that's a PITA on its own.

Comment on lines 15 to +17
"predist": "npm run clean",
"dist": "npm run build --workspaces",
"postdist": "npm run lint && npm run test && npm run docs --workspaces",
"postdist": "npm run lint && npm run test && npm run docs",
Copy link
Member Author

Choose a reason for hiding this comment

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

@flodolo These three commands are each run on npm run dist, see: https://docs.npmjs.com/cli/v8/using-npm/scripts#pre--post-scripts

Copy link

Choose a reason for hiding this comment

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

See my last comment: it doesn't work, I suspect because it stops after running tests.

Can you try on a clean clone?

Copy link

Choose a reason for hiding this comment

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

To confirm, this works

"postdist": "npm run lint;npm run test;npm run docs",

Copy link
Member Author

Choose a reason for hiding this comment

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

in a fresh clone, running

npm install
npm run dist

completes successfully for me, and produces the docs at html/. If the tests run but the next command (npm run docs) doesn't, that would indicate that the tests have failed. Can you check if there's any errors being logged from them? Even if there isn't, could you verify that running

npm run test
echo $?

prints a 0 to indicate success?

Copy link

Choose a reason for hiding this comment

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

It prints 2. Looks like dates are locale dependent (my system is set to Italian)?

  596 passing (172ms)
  3 pending
  2 failing

  1) Temporal support
       Temporal.Instant
         direct interpolation:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ '1/1/1970, 1:00:00 AM'
- '01/01/1970, 01:00:00'
      + expected - actual

      -1/1/1970, 1:00:00 AM
      +01/01/1970, 01:00:00

      at Context.<anonymous> (file:///Users/flodolo/Desktop/fluent.js/fluent-bundle/test/temporal_test.js:50:14)
      at process.processImmediate (node:internal/timers:483:21)

  2) Temporal support
       Temporal.Instant
         run through DATETIME():

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ '1/1/1970, 1:00:00 AM'
- '01/01/1970, 01:00:00'
      + expected - actual

      -1/1/1970, 1:00:00 AM
      +01/01/1970, 01:00:00

      at Context.<anonymous> (file:///Users/flodolo/Desktop/fluent.js/fluent-bundle/test/temporal_test.js:54:14)
      at process.processImmediate (node:internal/timers:483:21)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ping @rkh, see above. This is a regression from #636.

Copy link
Contributor

Choose a reason for hiding this comment

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

On it, looks like the JS environment produces different output for Intl.DateTimeFormat. Can prep a PR that handles this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@eemeli fwiw I cannot run npm run build/tsc atm, getting this:

src/scope.ts:72:7 - error TS2578: Unused '@ts-expect-error' directive.

72       // @ts-expect-error This is fine.
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It works if I remove that line (though I do get TypeScript LSP complaining about the line of code then)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

You might need to do an npm install as the lockfile TS version has been updated.

@eemeli eemeli merged commit b5dcf6a into main Mar 25, 2025
3 checks passed
@eemeli eemeli deleted the typedoc-update branch March 25, 2025 14:23
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.

3 participants