Attempt to improve support for using the PDF.js builds with Vite #18395
Conversation
Similar to Webpack there's apparently other bundlers that will not leave `import`-calls alone unless magic comments are used. Hence we extend the builder to also append `/* @vite-ignore */` comments to `import`-calls, in order to attempt to improve support for using the PDF.js builds together with Vite. This patch also renames `__non_webpack_import__` to `__raw_import__` since the functionality is no longer bundle-specific.
|
Closing for now, since no one seem to actually need or want this. (Note that down-voting my comments really isn't helping and quite frankly makes me even less motivated to land this.) |
This comment was marked as resolved.
This comment was marked as resolved.
|
Can we revisit this? Many libraries depend on this one and for vite users, this is needed. |
@Foxandxss Please read all of #18395 (comment), from which I'm quoting:
|
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
By what metric does this need to be justified? Likes on the issue/PR? |
At least one user willing to sponsor this work, as previously mentioned.
Yes, that's the case here since this is a feature that isn't needed in the Firefox PDF Viewer nor in the PDF.js library itself, and as mentioned everything adds maintenance/support burden. (Just because something is open-source doesn't mean that it's fair to expect that you can essentially assign more unpaid work to other people.) Please note that I'm one of the core contributors that help keep the PDF.js library working, so you could view this as an opportunity to sponsor someone whose work you depend upon. (Finally, note that down-voting my comments really isn't helping and quite frankly makes me even less motivated to land this.) |
|
Can someone clarify for me the implications here? We have been considering moving a project to use vite but if we cannot use pdfjs it is a deal breaker. Are there no workarounds? If we want to use vite does that mean we have no way of using pdfjs? |
As-is, it seems that Vite will error on
The solution is to, as mentioned multiple times already, sponsor me such that this PR will actually land since it's implementing the solution suggested in the Vite error message. |
This comment was marked as spam.
This comment was marked as spam.
@Snuffleupagus I have thought long and hard about how I wanted to approach this and comment here. I have been considering this for a while now and after talking to several people I trust and value in the open source community, I think it is appropriate to question the approach here. Especially given Vite's popularity, this does not feel like it is a "feature" that should be "paywalled" as @dzonatan pointed out. This is simply a necessary "patch" to work properly with one of the most popular build tools. I truly appreciate yours (and others) contributions to this library, but it feels a bit against the spirit of open source licensing to hold a PR hostage to someone paying an individual contributor (on a community library). As an example, what happens if someone pays you to "maintain" this patch moving forward, but you leave the project next week? I would be much more understanding if this was a large more niche feature that someone wanted implemented and offered a sort of bounty on it for a contributor to pick up. But for something as seemingly small as this PR (less than 30 lines of code changed over 10 files for a build step), and the fact that you already completed the PR and then held it hostage in a way, telling us if we want it merged we need to pay you... this is the part that doesn't feel like the open source spirit. Again, I appreciate and respect all you have done and continue to do for this library, but feel it is appropriate to share my opinion on this topic and kindly request that you reconsider. Best, |
|
FYI, opensource never meant that people are working for free. |
I 100% agree. And I understand how open source contributions can often go unnoticed and under appreciated which can be frustrating. My issue is more about the simple nature of this particular PR and the fact it was completed and then announced "if anyone would like this merged, please reach out to discuss payment". Additionally this isn't just a "nice to have" feature, it is something that a large chunk of users need in order to be compatible with one of the most popular build tools (vite). Again, I appreciate and respect all you all do for this library and want to make that clear. I thought long and hard for several months on this topic before deciding to comment, so this isn't meant to be a shortsighted complaint but an honest request for discussion and reconsideration. Thanks. |
|
Again the pdf.js goal is to be the Firefox pdf viewer: nothing else which means that we don't care about anything else. |
I can understand this perspective more if a user like myself submitted the PR, but this was drafted by @Snuffleupagus
The problem here is that a maintainer drafted the PR, users like myself said it would be helpful, and then it was cancelled because no one wanted to pay for a PR that was already completed.
Totally agree that this happens and it is unfortunate. If folks are successful monetarily from utilizing pdf.js I believe they should give back / contribute.
Unfortunately I am building a free product right now that isn't even in alpha testing yet. Certainly if I am successful and am able to monetize in some way down the road I will be happy to contribute monetarily to pdf.js as well as contribute development time if the opportunity and need arises.
@calixteman I would be happy to re-draft and submit this PR and take ownership of it moving forward. Is this something you would accept? |
Shouldn't this be noted somewhere in the |
@calixteman @Snuffleupagus can you kindly let me know your thoughts and you would support this? |
No, since that doesn't at all address the increased future maintenance burden that this feature would add (more work that'd essentially be forced upon me). Please keep in mind that once a feature is merged you're essentially "stuck" maintaining it[1], and you just can't rely solely on occasional contributors to handle that. Besides, someone still has to review and test any patches which also takes time. [1] Something that e.g. the TypeScript definitions have unfortunately shown repeatedly, since despite a large number of users wanting that feature it has on many occasions been difficult for us to get help with TypeScript-related problems. |
|
@JHarrisGTI wonderful news! Thank you so much!! |
Similar to Webpack there's apparently other bundlers that will not leave
import-calls alone unless magic comments are used. Hence we extend the builder to also append/* @vite-ignore */comments toimport-calls, in order to attempt to improve support for using the PDF.js builds together with Vite.This patch also renames
__non_webpack_import__to__raw_import__since the functionality is no longer bundle-specific.Fixes #17245 (comment)
Please note: This will further increase the maintenance/support burden of the general PDF.js library, for a feature that's completely unused within the project itself.
Hence, if you'd like to see this patch landed, please contact me privately to discuss sponsoring this work (email address can be found in my GitHub profile).