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 types #33

Merged
merged 7 commits into from
Jan 6, 2019
Merged

Add types #33

merged 7 commits into from
Jan 6, 2019

Conversation

Rokt33r
Copy link
Member

@Rokt33r Rokt33r commented Dec 15, 2018

I think type VFileCompatible = VFile | VFileOptions | VFileContents should be in here. So after this pr is merged, I'll make another pr to discard it from unified.

@wooorm
Copy link
Member

wooorm commented Dec 15, 2018

Nice! 👍

As the previous two type additions (vfile-message, unified) caused some of the ecosystem to break, I’d first like to figure out how to prevent that from happening before adding this.

@Rokt33r
Copy link
Member Author

Rokt33r commented Dec 15, 2018

Absolutely!

Copy link
Member

@wooorm wooorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but:

  • I’d first like unist-util-stringify-position to receive types as well
  • And maybe we need to change some other things in a major as well?

@ChristianMurphy
Copy link
Member

maybe we need to change some other things in a major as well?

What do you have in mind @wooorm?

@wooorm
Copy link
Member

wooorm commented Dec 23, 2018

@ChristianMurphy Nothing really? I think VFile works pretty well. We could look at adding a “base” instead of a “cwd“, similar to Gulp (vfile/ideas#4), tho?

I do know that adding types to an API surfaces some weirdness, so if @Rokt33r finds things that are weird or hard to handle, we could change them now.

package.json Outdated Show resolved Hide resolved
@Rokt33r
Copy link
Member Author

Rokt33r commented Dec 24, 2018

I do know that adding types to an API surfaces some weirdness, so if @Rokt33r finds things that are weird or hard to handle, we could change them now.

I don't' have any critical issues. But I sometimes thought it would be nice to have .freeze() method or immutable-vfile module.

@Rokt33r

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Jan 2, 2019

This PR is good to go but unist-util-stringify-position needs to be updated with types first!

@wooorm wooorm merged commit 3e5bc5e into vfile:master Jan 6, 2019
@wooorm
Copy link
Member

wooorm commented Jan 6, 2019

Released! I’d like to add types to the rest of the vfile packages next, before we move on to unified and other things.

Here’s my proposed order (taking dependencies into account, for the needed major bumps):

zero deep:

  • vfile-statistics
  • vfile-location
  • to-vfile
  • vfile-sort
  • vfile-is
  • vfile-reporter-folder-json
  • vfile-reporter-json

one deep:

  • vfile-to-eslint (vfile-statistics)
  • vfile-reporter (vfile-statistics)
  • vfile-find-up (to-vfile)
  • vfile-find-down (to-vfile)

two deep:

  • vfile-reporter-pretty (vfile-to-eslint)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☂️ area/types This affects typings 💪 phase/solved Post is done 🧑 semver/major This is a change 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

None yet

3 participants