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

Remove examples that suggest getting type from field.constructor.name #1287

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rmacklin
Copy link

@rmacklin rmacklin commented Jul 29, 2022

What?

This PR remove some code-comment examples that suggest getting type of a PDF field via field.constructor.name.

Why?

It is not safe/stable to rely on .constructor.name in general because minification will mangle the names of constructor functions (to make them as short as possible).

This has confused several folks before, as seen in these issues:
#736
#755
#933
#1089

As such, I think we should remove these examples from the codebase.

How?

I've simply removed the code that uses .constructor.name from these examples

Testing?

No testing is necessary for this PR since it only modifies two code comments.

New Dependencies?

No.

Screenshots

N/A

Suggested Reading?

No, but the reading is not relevant for this PR since it only modifies two code comments.

Anything Else?

No

Checklist

The rest of the checklist is not relevant for this PR since it only modifies two code comments.

  • I added/updated unit tests for my changes.
  • I added/updated integration tests for my changes.
  • I ran the integration tests.
  • I tested my changes in Node, Deno, and the browser.
  • I viewed documents produced with my changes in Adobe Acrobat, Foxit Reader, Firefox, and Chrome.
  • I added/updated doc comments for any new/modified public APIs.
  • My changes work for both new and existing PDF files.
  • I ran the linter on my changes.

It is not safe/stable to rely on `.constructor.name` in general because
minification will mangle the names of constructor functions (to make
them as short as possible).

This has confused several folks before, as seen in these issues:
Hopding#736
Hopding#755
Hopding#933
Hopding#1089

As such, I think we should remove these examples from the codebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant