-
Notifications
You must be signed in to change notification settings - Fork 79
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
Conversation
There was a problem hiding this 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
There was a problem hiding this comment.
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?
Correction: it looks like you need to run |
There was a problem hiding this 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.
"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", |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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",
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
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.