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

Create Feature image to base64 #81

Merged
merged 14 commits into from
May 14, 2023

Conversation

that-ambuj
Copy link
Contributor

@that-ambuj that-ambuj commented May 10, 2023

Hi @souvikinator, I have built the image link to base64 feature as we discussed in #60 with unit tests too but there are a few issues that I have faced:

  • Base64 format is not widely supported by many markdown parsers and renderers. I have tried Hugo, Github and markdown-it markdown parser.
  • The conversion performed using this features is very tricky and should be used carefully as some links return a javascript file redirect(like https://picsum.photos) the browser(not sure how this workes in node js) run instead of a raw image bytes; in that case the javascript code gets encoded to base64. PS: this issue is irrelevant as redirects are followed by node-fetch but there still might be some uncertainties.
  • People usually prefer images to be downloaded as files or permanently hosted in a storage bucket when using this library instead of base64, because of the aforementioned lack of support by popular markdown parsers and static-site generators.

I think it is understandable how base64 does not work with major markdown parsers because of how tricky and error prone this method of rendering images is(in case of point 2, it would produce a corrupted image, and the data is not guaranteed to be from an actual image) and the amount of challenge this feature produces is not worth the effort and a very small community trying to use this feature. Instead, I propose you that we work on temporary s3 links and already unsupported base64 data being emitted by this library.

@souvikinator
Copy link
Owner

the javascript code gets encoded to base64

That doesn't sound good and may lead to certain vulnerabilities. However I agree with your proposal, we can make it Notion's temporary s3 links exclusive and parse other links to regular markdown images syntax.

src/notion-to-md.ts Outdated Show resolved Hide resolved
src/types/index.ts Outdated Show resolved Hide resolved
@that-ambuj
Copy link
Contributor Author

Hi, the last two commits fix the base64 not being rendered by the markdown parsers and base64 images are now working. I'll also add a similar fix for the base64 data(coming from notion) that does not need conversion. To my surprise, the redirected images are now working as I've tested the base64 conversion with a picsum.photos link. I don't think there'll be any javascript embedded or executed in the base64 image data as node-fetch handles redirects very well.

@that-ambuj
Copy link
Contributor Author

Hey @souvikinator, I've merged the latest changes in the feature branch. There are no merge conflicts, everything is tested and ready to merge!

@that-ambuj
Copy link
Contributor Author

that-ambuj commented May 13, 2023

For reference, there is a page on MDN about everything you need to know about base64 images and data: prefixed urls: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs . Keep in mind this is a modern feature(with respect to IE) and the support for this is listed at the bottom of the aforementioned page.

@souvikinator
Copy link
Owner

Awesome! Thanks for the contribution. This really is an amazing feature. Should be live in upcoming major update.

@souvikinator souvikinator merged commit a1e96b7 into souvikinator:master May 14, 2023
@that-ambuj
Copy link
Contributor Author

that-ambuj commented May 14, 2023

It was my pleasure to work on this project! I'm looking forward to adding many more features to this project which you'll see in the future issues and PRs that I open.

Please also update the issues mentioning this PR and #60.

@that-ambuj that-ambuj deleted the feature-image-to-base64 branch May 14, 2023 07:13
@souvikinator souvikinator linked an issue May 14, 2023 that may be closed by this pull request
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.

Behaviour for uploaded images?
2 participants