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

deps: Switch docz-plugin-css implementation #418

Merged
merged 3 commits into from
Mar 7, 2022

Conversation

matheusd
Copy link
Member

@matheusd matheusd commented Feb 2, 2022

The current version of docz-plugin-css uses node-sass v4, which is
incompatible to node v16+.

Given that pi-ui does not use any sass files, this commit switches the
docz-plugin-css dependency to a forked version which drops node-sass and
is thus compatible to node v16+.

@matheusd
Copy link
Member Author

matheusd commented Feb 2, 2022

When testing, please double check the docz: scripts are also running successfully.

@tiagoalvesdulce
Copy link
Member

Thanks for the PR!

I'm getting the following error when running yarn docz:dev in this PR with a fresh install of dependencies:

TypeError: One and only one of the "port", "server", or "noServer" options must be specified

Steps to repro:

rm -rf yarn.lock && rm -rf node_modules && yarn cache clean
yarn
yarn docz:dev

The command docz:build works as expected.

Looking at your PR I can't figure out why this happened.
I'm really considering ditching docz and going for another option to write pi-ui's docs. Docz has been a pain to maintain. What do you think?

@matheusd
Copy link
Member Author

By that criteria (removing yarn.lock and rebuilding the dependency tree) it seems the current master is also broken.

Took me a bit, but I've been able to create resolution rules so that removing yarn.lock, then recreating it would yield a working docz:dev install. It really seems this version of docz isn't being maintained anymore, so at the very least, pi-ui should be upgraded to docz 2 if at all possible.

In any case, that's a much larger change then simply dropping sass support as I've done in this fork. Btw, I've updated the plugin-css fork to update the requirement to docz v1.2, so this also removes all dependencies to docz v0.

@matheusd matheusd force-pushed the master branch 2 times, most recently from fcefcd0 to 4bbbd93 Compare February 15, 2022 15:01
The current version of docz-plugin-css uses node-sass v4, which is
incompatible to node v16+.

Given that pi-ui does not use any sass files, this commit switches the
docz-plugin-css dependency to a forked version which drops node-sass and
is thus compatible to node v16+.
This should fix intermittent build errors.
@matheusd
Copy link
Member Author

matheusd commented Feb 16, 2022

Had to add a new commit, adding --network-concurrency 1 to the CI scripts due to intermittent errors.

See yarnpkg/yarn#6312

@tiagoalvesdulce
Copy link
Member

tiagoalvesdulce commented Feb 17, 2022

By that criteria (removing yarn.lock and rebuilding the dependency tree) it seems the current master is also broken.

Weird. We've never faced issues with docz:dev on master.

It really seems this version of docz isn't being maintained anymore, so at the very least, pi-ui should be upgraded to docz 2 if at all possible.

Yes, but docz v2 isn't that stable either. That's why I'll be writing the docs using another generator: #361 (comment)

This PR is important because it allows us to use node v16+ on politeiagui and decredtion right now. I tested locally with the following node versions and works fine:

v12.22.10
v14.19.0
v16.14.0

The only issue is that I'm seeing a lot of warnings I wasn't seeing before. Adding the following plugins to babel.config.js cleared the majority of them:

plugins: [
    ["@babel/plugin-proposal-class-properties", { "loose": true }],
    ["@babel/plugin-proposal-private-methods", { "loose": true }],
    ["@babel/plugin-proposal-private-property-in-object", { "loose": true }]
  ]

Can you add these plugins to this PR?

@victorgcramos
Copy link
Member

Tested @tiagoalvesdulce solution and it works. It still displays some warnings, but I was able to run it. Good to mention that warnings are only displayed on first docz:dev run.

@tiagoalvesdulce
Copy link
Member

I am merging this and will open a new PR to add:

plugins: [
    ["@babel/plugin-proposal-class-properties", { "loose": true }],
    ["@babel/plugin-proposal-private-methods", { "loose": true }],
    ["@babel/plugin-proposal-private-property-in-object", { "loose": true }]
  ]

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