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

fix(docz): dependency issues #1647

Merged
merged 14 commits into from
Aug 31, 2021
Merged

fix(docz): dependency issues #1647

merged 14 commits into from
Aug 31, 2021

Conversation

adbayb
Copy link
Contributor

@adbayb adbayb commented Aug 5, 2021

Description

A PR to address several high priority issues. It been tested by emulating new npm release through a local registry (please note that if you found several package version updates in some commits: it's totally normal and I've cleaned all tested version metadata to prepare a future release through this commit https://github.com/adbayb/docz/commit/24027115300b9b79d1811558044fb71357bd9fc5).

However, @renatobenks It could be great if you can publish a new alpha version to let issue authors confirms their issue resolution 😄

Main updates (fixes):

👉 Some other issue reporting might be solved as well (I have not tested other issue resolution).

Other updates (chore):

Opportunities (for future PRs)

  • Update all dependencies (especially gatsby ones)
  • Test with React 17 and update peerDependencies to accept newer version range

@renatobenks
Copy link
Member

renatobenks commented Aug 5, 2021

Awesome @adbayb, I'll gonna spend some time reviewing it, because I wanna stress some tests here, but great work!

So far I've seen, a couple issues are going to be fixed indeed!

@adbayb
Copy link
Contributor Author

adbayb commented Aug 10, 2021

Hello @renatobenks 👋 , any update on this? 🙏

@renatobenks
Copy link
Member

I'll be finishing some tests today, but once it's done, let's merge it, is that sounds good?

@adbayb
Copy link
Contributor Author

adbayb commented Aug 12, 2021

Sounds good, thank you @renatobenks 🙏

@adbayb
Copy link
Contributor Author

adbayb commented Aug 19, 2021

Any news @renatobenks ?

@renatobenks
Copy link
Member

Sorry for the big delay here, but I'm managing a new release for this one. I just need to ensure a few things first with @pedronauck, once I'm still not so sure about removing a few dependencies, even though it seems a good idea. But in general, so far so good in all my tests (which we need to automate ASAP from now on), and also, it'll be a great contribution for us all!

Thank you a lot @adbayb for the heads up on here 🙂

@renatobenks
Copy link
Member

hey @adbayb, are you willing to revert the removed dependencies? Let me know, because if not so, we could merge it right away, and right after that, evaluate those dependencies better before launching a new version. Otherwise, we'd be able to launch a new patch version, just after emerging this one. But in both cases, we wanna merge this PR.

@adbayb
Copy link
Contributor Author

adbayb commented Aug 27, 2021

Hello @renatobenks 👋 , devDependencies are not removed: they are hoisted at the monorepo level (no impact consumer side since dev dependencies are not installed).
But I can rollback it if you're more confortable with 👍

@renatobenks
Copy link
Member

Actually, I'm concerned about the "unused" dependencies, from dev or not, but let me list all of them for you, so yeah, I'd feel more comfortable with you rolling back, to publish a new version.

Copy link
Member

@renatobenks renatobenks left a comment

Choose a reason for hiding this comment

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

Hey, here follows the placed changes to rollback

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
core/docz-core/package.json Show resolved Hide resolved
core/docz-core/package.json Outdated Show resolved Hide resolved
core/docz-core/package.json Show resolved Hide resolved
core/docz/package.json Show resolved Hide resolved
core/docz/package.json Show resolved Hide resolved
core/docz/package.json Outdated Show resolved Hide resolved
core/docz/package.json Outdated Show resolved Hide resolved
core/docz/package.json Outdated Show resolved Hide resolved
@adbayb
Copy link
Contributor Author

adbayb commented Aug 30, 2021

@renatobenks Done 90ab65d
Hope it'll help make your dependency review easier afterwards.

@renatobenks renatobenks merged commit 5bdca48 into doczjs:main Aug 31, 2021
@adbayb adbayb deleted the fix/dependencies branch September 1, 2021 08:08
@adbayb
Copy link
Contributor Author

adbayb commented Sep 2, 2021

@renatobenks Thank you for your review. Do you plan a release soon?

@renatobenks
Copy link
Member

Yeap, I'm releasing today a new alpha version to the next patch, v2.3.2 respectively, and in the meantime, you can be using this one until having the final patch version, which we're pretending to launch as soon as we finish a couple of automated tests that we're working on.

Let us know if you have any issues towards this end.

@adbayb
Copy link
Contributor Author

adbayb commented Sep 6, 2021

Thank you @renatobenks 🙏 .
Unfortunately, I can't find the newly alpha. It seems that the latest one is the 2.0.0-rc.23 (publication date: 02/09/2019). Same for the next tag (the alpha.0 is not new).
What's the right version please? Thanks in advance.

Capture d’écran 2021-09-06 à 16 46 57

@renatobenks
Copy link
Member

renatobenks commented Sep 10, 2021

Hey @adbayb, this upcoming version it's on https://www.npmjs.com/package/docz/v/2.3.3-alpha.0 (https://github.com/doczjs/docz/tree/v2.3.3-alpha.0), still as alpha because I would like to get some people's feedback about how it goes, before launching it as a final patch, so please, let me know how it moves, that I'll be making some extra work for validation from here as well.

@adbayb
Copy link
Contributor Author

adbayb commented Sep 13, 2021

Thanks @renatobenks , I've tested the alpha release: it works just fine in our company codebase 🚀

@drewlustro
Copy link

This is a lovely patch @adbayb, thank you for contributing it!

The react-docgen upgrade, in particular, made Prop table generation significantly more reliable for us.

Thanks again ❤️

@renatobenks
Copy link
Member

renatobenks commented Oct 13, 2021

We're about to release a new alpha version to that patch because I found a significant problem for those who are not using typescript and have it as not enabled.

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