packaging improvements about workers-sdk HOT 15 OPEN

I don't think performance is a consider when using async/await vs Promise. It won't have any noticeable impact in your worker. I would privilege the one that's easier to write/maintain.

On the other hand size is a concern, webpack should be able to swap out mime full for the lite version at build time. I'm concerned that using the lite version by default would break existing users.

from workers-sdk.

so we may keep things like await syntax which may help reduce final worker size (and probably more v8 optimization by using modern syntax)

Yes, I do agree on the polyfill for async/await is not necessary since Cloudflare Workers already provides the support for it. But IMHO I prefer kv-asset-handler to be written in traditional Promise instead of async/await: although v8 improves the performance of async/await quite a lot, it is still slower than traditional Promise.

mime has a lite entry (25 kB vs 9 kB) which is containing all of standard mimes. This can significantly reduce bundle size. You may import from mime/lite

Although full mime is a bit large, the bundle size will not affect so much since Cloudflare has the 1 MiB size limit for the script.

from workers-sdk.

I may be wrong, but I am not sure there is a great appetite for a rework at CF on this one. I did pretty much the entire refactor you are describing in the PR I submitted above (replicating the functionality of mime/mime-lite to remove the dependency and make it more maintainable, ES modules etc) but it ends up being quite a substantial rewrite for questionable(?) gain.

In a sense, from our (customer) perspective, Pages has somewhat superseded kv-asset-handler (although, based on the defaults for Pages, I strongly suspect it is running kv-asset-handler or a modified version/derivative of it).

If Greg (or anyone else at CF) wants to revisit (some, or all of) the changes in my PR, or generally discuss, I'm happy to contribute as needed.

from workers-sdk.

Yes, I do agree on the polyfill for async/await is not necessary since Cloudflare Workers already provides the support for it. But IMHO I prefer kv-asset-handler to be written in traditional Promise instead of async/await: although v8 improves the performance of async/await quite a lot, it is still slower than traditional Promise.

For sure. And since currently there are two await calls, splittting branches with two helper functions shouldn't be hard... Still there are more polyfills in current dist than async/await that not using transpilation could make it more faster and easier to tree-shake.

Although full mime is a bit large, the bundle size will not affect so much since Cloudflare has the 1 MiB size limit for the script.

That's right! But we are using it right now in a server-side-rendering solution. Size impact of each dependency matters when using kv-asset-handler. (it is currently fixed by an alias at bundler level)

(i might be wrong at this but a larger DB may also cause slighlty longer for mime lookup)

from workers-sdk.

That's right! But we are using it right now in a server-side-rendering solution. Size impact of each dependency matters when using kv-asset-handler. (it is currently fixed by an alias at bundler level)

Since it is possible to serve "uncommon" files through Cloudflare Workers, at least we should make mime lite/full configurable. But then it becomes unsure whether dynamic import could be tree-shaken correctly.

from workers-sdk.

At least we should make mime lite/full configurable

Agreed. It might be somehow handled similar at wrangler level for aliasing to lite/full

Just to clarify this is what mentioned about lite:

There is also a "lite" version of this module that omits vendor-specific (*/vnd.*) and experimental (*/x-*) types. It weighs in at ~2.5KB, compared to 8KB for the full version

(sizes are in gzip)

from workers-sdk.

Fully agree with @xtuc points.

• Using async/await makes code readable and letting v8 engine to optimize without tackling it
• Directly using mime here is probably safest. We may implement alias with wrangler and add as a tip to documentation as a best practice if do not want to serve files with vendor/exp mime

from workers-sdk.

Agree with much of this, as we are using an adapted version of kv-asset-handler in this context. We are part of the Durable Objects beta, so we've been reworking some of our dependencies to ES Modules where it will clean up the build by removing bundler boilerplate. For kv-asset-handler, this required a tweak of tsconfig.json and a quick rewrite/cleanup of the relevant bits of mime into TypeScript (in the context of how kv-asset-handler uses mime, it's just a wrapper around mime-db and the maintainer appears to prefer compatibility over modern JS structures).

If you want, I can formalise our branch with with a few tests, create a secondary build path for an ES module output and make the new 'mime wrapper' easy to instantiate with a different/subset of the mime-db (as per the lite version of mime) and submit a pull request?

from workers-sdk.

Great discussion here, thanks all. I've got a few changes in cloudflare/kv-asset-handler#179 that help here, though I'd encourage any feedback and suggestions people have. That PR sets a files config for NPM publishing, to reduce the unnecessary publishing of configs, etc. as well as tweaks the TypeScript configuration just a little bit, to reduce the unnecessary polyfilling of generators (for async/await).

There's probably a lot more we could do with our build chain and/or TypeScript to further reduce the output size. With the recent changes I made, the only remaining polyfill really seems to be for async/await, which we could also likely remove in the future with proper ESM support.

from workers-sdk.

cloudflare/kv-asset-handler#254 increased the TypeScript build target to ES2017 which removed the final async/await polyfill.

I believe the only thing left in this issue now is any further discussion around mime/mime/lite.

from workers-sdk.

Size of my bundle increased from 5kb to 17kb after installing this package.
Execution time increased by ~10ms as well. (basing on Time to First Byte (TTFB) stat)

Not terrible but not cool either.

from workers-sdk.

I'm currently finalising changes on my fork (based around ES modules) which I will submit pull requests from soon. Certainly the mime stuff has a load of uncommon mime types which are unnecessary for the average user and could be significantly reduced in size. I would also guess a high percentage of people use wrangler rather than dynamically populating the asset manifest, in which case it should be possible to narrow down the mime list to the specific set required per user (although I can't be certain how much impact that would have on TTFB etc).

from workers-sdk.

For example, we narrow down our mimes and other types at build-time: https://github.com/nuxt/framework/blob/main/packages/nitro/src/rollup/plugins/static.ts#L10-L28

So any removal of these from the runtime package would be very nice 😁

from workers-sdk.

That seems like a sensible approach - only at publish/build is there a chance to narrow the types to the specific subset needed.

Can't comment on what direction the CF people might want to go, but it would seem that this is probably a situation where the user has to take some responsibility for narrowing down the types - would guess that there are quite a few edge cases which would be difficult to cover if this package was overly simplified.

Will give some thought as to whether there is an easy way of having a user supplied mime list which, if using something like rollup, would remove the default list and thereby reduce the package size.

from workers-sdk.

Three years have passed since the issue was first raised, and yet the mime package remains the main factor contributing to the large bundle size.

Even when with mime/lite, the size of the bundled/minified package is still substantial at 12.1 KiB.

A noteworthy alternative is the mrmime package (whose maintainer is a former Cloudflare Staff—kudos to him!) that builds upon the mime/lite. After bundling and modification, the size of the mrmime can be reduced to 10.8 KiB, which is still a lot.

The data sources for both the mime and the mrmime packages are quite outdated and no longer actively maintained. The last update to the mime-db was a year ago, and [numerous issues remain unaddressed](https://github.com/jshttp/mime-db/issues.

Moreover, a larger worker dist size leads to slower bootstrap times, higher Time To First Byte (TTFB), and overall poorer performance (as mentioned previously where their workers' TTFB has increased by 10ms).

from workers-sdk.