-
Notifications
You must be signed in to change notification settings - Fork 878
feat: Remove Node.js runtime dependencies from docfx tools #10066
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
feat: Remove Node.js runtime dependencies from docfx tools #10066
Conversation
5e08bf4 to
53ef11f
Compare
|
When using
So I've reverted code to |
a32b02f to
0b7b04b
Compare
yufeih
left a comment
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.
Thank you @filzrev !
|
Hi @yufeih, is it expected that everyone has to install the node.js locally (and on CI) even if we don't use any PDF related feature? |
|
Node.js installation is required when running following commands.
It might be better to modify I'll try to create PR later. |
|
I use the default command with Could it be a possibility to not require Node.js if PDF output is disabled? |
|
Workaround is simply by setting
My build then succeeded as it used to. Or install node.js even if it's not required (unless you are using pdf). |
|
I've created PR(#10406) to fix a reported problem with the default |
|
FWIW in the meantime, RUN PLAYWRIGHT_NODEJS_PATH="workaround https://github.com/dotnet/docfx/pull/10066#issuecomment-2486269657" docfx tech/docfx.json --disableGitFeatures=trueworks in my Dockerfile. Thanks @JanVargovsky and of course @filzrev for the super fast fix! |
This PR intended to resolve #9396.
Background
Playwright
1.41or later supports following environment variables. (It's not documented though)PLAYWRIGHT_NODEJS_PATHPLAYWRIGHT_DRIVER_SEARCH_PATHSo it can remove Node.js runtime from
docfxpackage.Instead. it can use locally installed Node.js runtime instead.
By this changes.
docfxpackages size can be reduced from208MB->56 MBBREAKING CHANGES
This PR remove
Node.jsruntime bundles.So it may introduce breaking changes for some users who using docfx without Node.js installation.
(e.g. docker image users)
I'll try to fix official docker image later.
https://github.com/dotnet/docfx/blob/main/Dockerfile