search

fastify / fastify-swagger-ui

Serve Swagger-UI for Fastify
MIT License
125 stars 38 forks source link

Large package size #121

Closed SeanReece closed 5 months ago

SeanReece commented 5 months ago

Prerequisites

Fastify version

4.26.0

Plugin version

2.1.0

Node.js version

20

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

14.2

Description

It appears as though the @fastify/swagger-ui package is quite large. ~2x our next largest package.

image

Looks like this is because the swagger source map files are shipped with the package. ~2.8MB in source maps.

image

Is it necessary to include source maps in the published package?

This might not seem like a big issue for server side dependencies, but this installing larger packages does increase build times and deploy times, if only slightly.

Of course using something like clean-modules does remove .map files but that doesn't change the fact that npm install still has to pull the .map files first.

Steps to Reproduce

> npm install @fastify/swagger-ui > ls -lhS ./node_modules/@fastify/swagger-ui/static

Expected Behavior

No response

mcollina commented 5 months ago

I think we can safely remove the map files. Would you like to send a PR?

SeanReece commented 5 months ago

@mcollina I would love to! Just wanted to confirm first.

Uzlopak commented 5 months ago

I am not a frontend dev, but for some reason when opening developer tools in chrome it loads the map files. As I am not a frontend dev, I never invested time to understand why it does this. Maybe some frontend DX debugging feature.

SeanReece commented 5 months ago

@Uzlopak If you look at the bottom of the source files (swagger-ui-bundle.js for example), you'll see a comment that tells chrome where to find the source map //# sourceMappingURL=swagger-ui-bundle.js.map. Testing this in chrome it gives me this error only when trying to view the source from "Sources" tab. I don't see any 404s or console errors. For me, it doesn't seem to eagerly try to load them (at least I don't see anything from the network tab)

image

It doesn't make much sense to me for swagger-ui-dist to provide source files in the first place 🤔 If I wanted to debug swagger-ui I think I would pull the swagger-ui repo and run it from there. 😄

mcollina commented 5 months ago

In case we should remove those statements as well.

Uzlopak commented 5 months ago

@mcollina

Is there a reason to vendorize swagger-ui?

mcollina commented 5 months ago

We need to process it https://github.com/fastify/fastify-swagger-ui/blob/master/scripts/prepare-swagger-ui.js as well as it used to break between versions, so it would have to be pinned.

Uzlopak commented 5 months ago

I think the only reason we are vendoring it, is because we could be run in the possbility that swagger-ui is not installed on node_modules root?!

mcollina commented 5 months ago

We are computing hashes: https://github.com/fastify/fastify-swagger-ui/blob/479a4c2d10fb344d1b38af310be8a408b3deaeb0/scripts/prepare-swagger-ui.js#L65

Uzlopak commented 5 months ago

@SeanReece

What i like about this issue is, that even my comment, which was just about a small sideeffect of loading map files, got the necessary attention.

This is the spirit of OSS.

Thank you @SeanReece