Skip to content

Conversation

@KevLehman
Copy link
Member

Proposed changes (including videos or screenshots)

Issue(s)

https://app.clickup.com/t/vb7dwv

Steps to test or reproduce

Further comments

  • The library was one of the only ones with no external dependencies (a lot rely on exiftool for doing the removal)
  • Library was almost the only one with stream support
  • It didn't work only for JPEGs, although it's the most common format that includes exif data
  • Metadata from other file types (pdf, video files) is not removed. The good part is that videos/files containing metadata are far less common than images.
  • If we want to add support for all image/video types, we would need a combination of exiftool + ffmpeg

@KevLehman KevLehman requested a review from a team May 14, 2021 17:59
@KevLehman
Copy link
Member Author

Huh, interesting, I didn't know we typechecked the packages too 🤔

@sampaiodiego
Copy link
Member

wut.. I didn't know as well 🤔 weird

@KevLehman
Copy link
Member Author

@sampaiodiego I can copy the code from the library into RC, however 🤔 it bugs me on why it's trying to typecheck the module.

@sampaiodiego
Copy link
Member

@KevLehman can you maybe take a look if that is the intended behavior of tsc? or if there is a flag we can provide to not do that? or maybe something on our tsconfig.json

@KevLehman
Copy link
Member Author

@sampaiodiego it's weird, indeed. It looks like it's related to the import statement. Basically, when you import something, tsc will see it and try to compile it (ref microsoft/TypeScript#41883 (comment))

Changing the import for a require fixes the issue, but it doesn't look like a clean solution 😬 what do you think?

@sampaiodiego sampaiodiego changed the title Remove exif metadata from files [NEW] Remove exif metadata from uploaded files May 21, 2021
@sampaiodiego sampaiodiego merged commit 1600bcb into develop May 21, 2021
@sampaiodiego sampaiodiego deleted the fix/exif-meta branch May 21, 2021 02:35
@sampaiodiego sampaiodiego mentioned this pull request May 28, 2021
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.

3 participants