search

gtm-nayan / svelte-pdfjs

https://gtm-nayan.github.io/svelte-pdfjs
MIT License
31 stars 8 forks source link

Rethink onMount abstraction #4

Open gtm-nayan opened 2 years ago

gtm-nayan commented 2 years ago

Currently, the Document and Page components dynamically import internal Document and Page components, for compatibility with SvelteKit, this was done to avoid having to do it in the consumer.

However, people using the lib would probably not use the Document and Page components directly, instead create their own PDFViewer component using these. In which case, dynamically importing them in the consumer wouldn't really be that big of a deal.

So, is it really necessary to do the dynamic imports within the library?

gtm-nayan commented 2 years ago

Actually nvm, forgot about this. Keeping this open for if anything changes.

gtm-nayan commented 2 years ago

For, Document it seems that something recently changed somewhere either in Vite, PDFJS or the adapters which now allow PDFJS to be imported statically without any onMount shenanigans,

however, the worker initializer must not be called on the server otherwise it'll crash, so Document needs to be wrapped in a {#if} browser check for sveltekit. This is a lot more concise than onMount and it allows us to avoid needing to have a wrapper component, combined with possible use cases mentioned in the first comment, I think this is a worthwhile tradeoff

Will keep the issue open still for the abstraction in Page