Skip to content

[HOLD] Codemods - #6

Open
Hypercubed wants to merge 4 commits into
stephanrauh:bleeding-edgefrom
analyst-one:codemods
Open

[HOLD] Codemods#6
Hypercubed wants to merge 4 commits into
stephanrauh:bleeding-edgefrom
analyst-one:codemods

Conversation

@Hypercubed

Copy link
Copy Markdown

This is a proof of concept.

I've created a codemod that converts all private methods and properties to public prefixed with a double underscore. In this PR I ran it only on web/pdf_find_controller.js but I also tried it on web/*.js. Testing in this repo looks ok; I'm sure more testing would be required. But before that some questions:

  • Is this something you are interested in?
  • Would you want to run it on all js files or just a select few?
  • How and when would you like to trigger the jscodeshift cli? npm run codemods?

FYI... to run the code mod the command is npx jscodeshift -t ./codemods/replace-private-methods.cjs {file}

@Hypercubed

Copy link
Copy Markdown
Author

I'm thinking this should be part of the build process somehow... so it doesn't make merging from upstream harder.

@stephanrauh

Copy link
Copy Markdown
Owner

That's a very good question. For me, private variables are annoying because I can't see their values when inspecting an object in the console. I don't think converting real private variables into private-by-convention attributes does any harm. Basically, making stuff private is chiefly motivated by improving maintainability.

What about the bundle size? How much does it grow? Or does the minified rename attributes that are private by convention?

BTW, you should offer your code mod to the pdf.js team, just the other way round: they spend a lot of time modernizing their JavaScript code.

@stephanrauh

Copy link
Copy Markdown
Owner

@Hypercubed As far as I remember, I've integrated your changes, but for some reason, this PR is still open. Can I close it?

@stephanrauh stephanrauh self-assigned this Sep 27, 2024
@Hypercubed

Copy link
Copy Markdown
Author

@Hypercubed As far as I remember, I've integrated your changes, but for some reason, this PR is still open. Can I close it?

@stephanrauh This was asking if you were interested in a PR to automate converting pdfjs privates to prefixed publics. If it doesn't help your process than feel free to close it.

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

Successfully merging this pull request may close these issues.

2 participants