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

Add support for WHATWG URLs #16

Closed
4 tasks done
aduh95 opened this issue Jun 9, 2021 · 6 comments · Fixed by #17
Closed
4 tasks done

Add support for WHATWG URLs #16

aduh95 opened this issue Jun 9, 2021 · 6 comments · Fixed by #17
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have

Comments

@aduh95
Copy link
Contributor

aduh95 commented Jun 9, 2021

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)

Subject

Node.js fs API supports URL for all fs operations, it'd be nice to add support for it

Problem

I'd expect the following to work:

import { toVFile } from 'to-vfile';
const doc = new URL('../hello.md', import.meta.url);
toVFile.readSync(doc, 'utf8');

Unfortunately, it's neither a string nor a buffer, no error is reported that I've done something forbidden but the VFile instance does not populate correctly.

Solution

to-vfile could check if the provided object is a URL object, and convert it to a path using fileURLToPath from node:url module.

Alternatives

to-vfile could throw an error if the provided object is a URL object, to let the user know they need to convert it themself:

import { fileURLToPath } from 'url';
import { toVFile } from 'to-vfile';
const doc = new URL('../hello.md', import.meta.url);
toVFile.readSync(fileURLToPath(doc), 'utf8');
@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jun 9, 2021
@wooorm
Copy link
Member

wooorm commented Jun 9, 2021

heyy!

I was recently thinking of the same thing, as I’m in the process of moving everything over.
Probably also to vfile, and maybe other places.

The error would be a quick fix. Supporting them would be nicer (although: vfile is often used in the browser, so not using url would be preferred, but this doesn’t concern to-vfile).
Last, and maybe more for the future, would be to use URLs internally too, and changing the methods currently on vfile to be URL-centric.

@wooorm wooorm added the 🙆 yes/confirmed This is confirmed and ready to be worked on label Jun 9, 2021
@github-actions github-actions bot added 👍 phase/yes Post is accepted and can be worked on and removed 🤞 phase/open Post is being triaged manually labels Jun 9, 2021
@github-actions

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Jun 9, 2021

@wooorm
Copy link
Member

wooorm commented Jun 9, 2021

@aduh95 Is this something you could work on?

@wooorm wooorm added 🗄 area/interface This affects the public interface 🦋 type/enhancement This is great to have 🧒 semver/minor This is backwards-compatible change help wanted 🙏 This could use your insight or help labels Jun 9, 2021
aduh95 added a commit to aduh95/to-vfile that referenced this issue Jun 10, 2021
@aduh95 aduh95 mentioned this issue Jun 10, 2021
5 tasks
wooorm pushed a commit that referenced this issue Jun 11, 2021
Closes GH-16.
Closes GH-17.

Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>
Reviewed-by: Remco Haszing <remcohaszing@gmail.com>
Reviewed-by: Merlijn Vos <merlijn@soverin.net>
Reviewed-by: Titus Wormer <tituswormer@gmail.com>
@github-actions

This comment has been minimized.

@wooorm wooorm added 💪 phase/solved Post is done and removed help wanted 🙏 This could use your insight or help labels Jun 11, 2021
@github-actions github-actions bot removed 👍 phase/yes Post is accepted and can be worked on 🙆 yes/confirmed This is confirmed and ready to be worked on labels Jun 11, 2021
@wooorm
Copy link
Member

wooorm commented Jun 11, 2021

Released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging a pull request may close this issue.

2 participants