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

Upgrade json-schema-to-typescript #7934

Closed
benmccann opened this issue Aug 28, 2024 · 8 comments
Closed

Upgrade json-schema-to-typescript #7934

benmccann opened this issue Aug 28, 2024 · 8 comments
Assignees
Labels

Comments

@benmccann
Copy link

Link to reproduction

https://npmgraph.js.org/[email protected]

Environment Info

3.0.0-beta.95

Describe the Bug

Upgrading would drop 14 dependencies:
https://npmgraph.js.org/[email protected] - 51 dependencies
https://npmgraph.js.org/?q=json-schema-to-typescript - 37 dependencies

Reproduction Steps

https://npmgraph.js.org/[email protected]

Adapters and Plugins

No response

@benmccann benmccann added status: needs-triage Possible bug which hasn't been reproduced yet v3 labels Aug 28, 2024
@AlessioGr AlessioGr self-assigned this Aug 28, 2024
@github-actions github-actions bot removed the status: needs-triage Possible bug which hasn't been reproduced yet label Aug 28, 2024
AlessioGr added a commit that referenced this issue Aug 28, 2024
@AlessioGr
Copy link
Member

Thank you! Closed by #7938

@benmccann
Copy link
Author

wow! amazing! thank you for the super fast turnaround on this

the other one I was going to ask about was pinot + pinot-pretty. I think that they're probably responsible for about 1/3 of the dependencies in payload 3, which seems like quite a lot for a logging library. I don't have a specific recommendation on that, but wonder if there might be any lighter-weight alternatives?

@AlessioGr
Copy link
Member

wow! amazing! thank you for the super fast turnaround on this

No, thank you for bringing this to our attention! Also just dug around our package.json a bit more - managed to also decrease the total install size of the payload package + its dependencies from 123 MB to 34 MB. The leaner we keep this package the better!

the other one I was going to ask about was pinot + pinot-pretty. I think that they're probably responsible for about 1/3 of the dependencies in payload 3, which seems like quite a lot for a logging library. I don't have a specific recommendation on that, but wonder if there might be any lighter-weight alternatives?

This one will be trickier. Definitely a breaking change since we expose our logger, including the pino options, to our users. This will be especially bad for users that add their own pino transports. For example, I'm using a pino transport to automatically upload my logs to axiom.

An alternative logging library would also need to produce JSON logs (which is essential for being able to filter logs). + pino claims to have a very low performance overhead - would have to make sure an alternative library doesn't decrease speed in favor of dependency count.

@benmccann
Copy link
Author

An easier one to tackle might be bcherny/json-schema-to-typescript#623. The author of that library was initially reluctant, but said if there was significant interest from others, they'd be open to making the change

I was also curious why pinned package versions are used as prefixing all packages with ^ may allow for better deduping

Also just dug around our package.json a bit more - managed to also decrease the total install size of the payload package + its dependencies from 123 MB to 34 MB.

Amazing!! Definitely tell your boss 😆 Seriously I looked at all the CMS and payload was already the lightest and I think it can get even further ahead, which will be a great line in the release announcement

This one will be trickier...

Thanks for sharing! That makes sense. Maybe the easier thing then would be to see if pino/pino-pretty could remove their dependency on readable-stream

@AlessioGr
Copy link
Member

An easier one to tackle might be bcherny/json-schema-to-typescript#623. The author of that library was initially reluctant, but said if there was significant interest from others, they'd be open to making the change
glob is responsible for 26 out of 37 dependencies for this library

Crazy! Hoping for that to be resolved soon. Lmk if I can help make a PR for this library, but I'd like to hear what the author has to say first regarding which glob library they prefer.

One thing we'll be looking into is replacing the react-select package from @payloadcms/ui - which is most of payload's admin panel. Looking at npmgraph, it's responsible for most of our total dependencies, which is insane given that it's just one single component.

I was also curious why pinned package versions are used as prefixing all packages with ^ may allow for better dedupin

We've experienced a lot of stability issues in our package.json due to carets. We've had packages we depend on release broken updates or make breaking changes (sometimes masked as bug fixes) in patch versions.
When this happened, fresh installs (or re-installs after deleting the lockfile) of payload just stopped working overnight—all because the caret pulled in the latest versions automatically.

To mitigate this, we've started specifying exact package versions to maintain better control over our dependencies. Well, we still face potential issues with peer dependencies of our dependencies, which we can't control, but this issue has been happening a lot less since.

Amazing!! Definitely tell your boss 😆 Seriously I looked at all the CMS and payload was already the lightest and I think it can get even further ahead, which will be a great line in the release announcement

Thank you! Yea a lot of effort has been put into keeping the core payload package as minimal as possible in 3.0. Users should be able to use it in any project they want without having to install a bunch of useless dependencies with it, and in any environment: classic servers, serverless, and in the future edge.

A good example that may be interesting for you given that you're a Svelte person: While payload's admin panel (payload + @payloadcms/ui + @payloadcms/next) is running on Next.js, you can simply install and run payload - which is the server-side of things - inside of a SvelteKit app.
And boom, you get all the benefits of running payload on the server and using its local API in the same app, without bloating your app with an entire "CMS".

Maybe the easier thing then would be to see if pino/pino-pretty could remove their dependency on readable-stream

We should definitely issue in their repo about this. The maintainer seems to be very conscious about performance and all that.
What could also be possible is utilizing an adapter pattern for the logging library, but that might be overkill and we haven't explored that yet

@benmccann
Copy link
Author

One thing we'll be looking into is replacing the react-select package from @payloadcms/ui - which is most of payload's admin panel. Looking at npmgraph, it's responsible for most of our total dependencies, which is insane given that it's just one single component.

That's great news! Yeah, that and deep-equal appears to be another big one, which can be replaced with either dequal or fast-deep-equal. I think qs could be replaced there as well. Here are some suggestions for that one: https://github.com/es-tooling/module-replacements/blob/main/docs/modules/qs.md. If there are others you wanted ideas for you could ask folks at https://chat.e18e.dev/

A good example that may be interesting for you given that you're a Svelte person: While payload's admin panel (payload + @payloadcms/ui + @payloadcms/next) is running on Next.js, you can simply install and run payload - which is the server-side of things - inside of a SvelteKit app.
And boom, you get all the benefits of running payload on the server and using its local API in the same app, without bloating your app with an entire "CMS".

Yeah, that sounds really interesting to explore! If you or anyone else were interested in talking about it at the Svelte Summit virtual conference in October, they're currently accepting ideas for talks for the next two weeks: https://www.sveltesummit.com/2024/fall (I don't see the deadline on the website, but believe it's Sept 14)

@AlessioGr
Copy link
Member

That's great news! Yeah, that and deep-equal appears to be another big one, which can be replaced with either dequal or fast-deep-equal. I think qs could be replaced there as well. Here are some suggestions for that one: es-tooling/module-replacements@main/docs/modules/qs.md. If there are others you wanted ideas for you could ask folks at chat.e18e.dev

Both deep-equal and qs are already replaced! (Be sure to check the beta (3.0) branch. main (= 2.0) still has those dependencies, but we're no longer concerned 2.0). In fact, we made the qs-esm library just to get rid of it hah. I think we probably got all the easy-to-replace, low-hanging fruit dependencies

Yeah, that sounds really interesting to explore! If you or anyone else were interested in talking about it at the Svelte Summit virtual conference in October, they're currently accepting ideas for talks for the next two weeks: sveltesummit.com/2024/fall (I don't see the deadline on the website, but believe it's Sept 14)

Sounds interesting, I'll bring this up with our in-house influencer @jmikrut !

Copy link

github-actions bot commented Sep 6, 2024

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2024
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants