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

[Type-definitions] Hack around the broken SVGGraphics import map (issue 16705) #16845

Closed
wants to merge 2 commits into from

Conversation

Snuffleupagus
Copy link
Collaborator

Previous attempts at "importing" the @typedef caused the JSDoc-generation to fail, however it seems that prepending it with @ignore causes JSDoc to completely skip the rest of the line while TypeScript will (seemingly) parse the @typedef as usual.
While both gulp jsdoc and gulp typestest pass with this patch, it's unfortunately nothing more than a crude work-around for the actual issue.

This way of "fixing" issue #16705 does feel very undesirable, for a number of reasons:

  • It does nothing to address the underlying issue, i.e. that the Type-generation doesn't understand the import maps, which means that the same problem could resurface somewhere else in the code-base next week/month.
  • It feels very arbitrary/brittle, since a future update of JSDoc and/or TypeScript could very easily break this hack.
  • It further increases the maintenance burden for the Type-generation, when the opposite would be desirable here. (Since it's already very difficult to get TypeScript users to actively help with maintaining this code.)

…ssue 16705)

Previous attempts at "importing" the `@typedef` caused the JSDoc-generation to fail, however it seems that prepending it with `@ignore` causes JSDoc to completely skip the rest of the line while TypeScript will (seemingly) parse the `@typedef` as usual.
While both `gulp jsdoc` and `gulp typestest` pass with this patch, it's unfortunately nothing more than a crude work-around for the actual issue.

This way of "fixing" issue 16705 does feel very undesirable, for a number of reasons:
 - It does nothing to address the underlying issue, i.e. that the Type-generation doesn't understand the import maps, which means that the same problem could resurface somewhere else in the code-base next week/month.
 - It feels very arbitrary/brittle, since a future update of JSDoc and/or TypeScript could very easily break this hack.
 - It further increases the maintenance burden for the Type-generation, when the opposite would be desirable here. (Since it's already very difficult to get TypeScript users to *actively* help with maintaining this code.)
@Snuffleupagus
Copy link
Collaborator Author

/botio preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/446d621c9ad6acb/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/240e63226e036b7/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/240e63226e036b7/output.txt

Total script time: 1.44 mins

Published

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/446d621c9ad6acb/output.txt

Total script time: 11.35 mins

Published

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Aug 17, 2023

In issue #16705 only a single TypeScript user has, so far, actually attempted to help debug/fix this problem.

Hence this unfortunately once again highlights the problem with the type-definitions, which is that many (or most) TypeScript users seem unable (or unwilling) to actively help maintain the types.
This is something that I was concerned about back when the type-generation originally landed, and experience has unfortunately shown that unpaid PDF.js contributors (such as myself) have been "forced" to spend quite a lot of time/effort on maintaining the type-definitions. (Given that the primary development target for the PDF.js library is the built-in Firefox PDF Viewer, keeping the library "working" in other browsers/environments takes much more time/effort than many users probably realize.)

With this in mind, if you're using the PDF.js library in a commercial setting and depend on working type-definitions, please consider https://github.com/sponsors/Snuffleupagus/

@stof
Copy link
Contributor

stof commented Aug 31, 2023

@Snuffleupagus I submitted a proper solution in #16890

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants