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

Rethink onMount abstraction #4

Open
gtm-nayan opened this issue Dec 22, 2021 · 2 comments
Open

Rethink onMount abstraction #4

gtm-nayan opened this issue Dec 22, 2021 · 2 comments

Comments

@gtm-nayan
Copy link
Owner

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
Copy link
Owner Author

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

@gtm-nayan
Copy link
Owner Author

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

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

No branches or pull requests

1 participant