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

feat: TS type information for remark-stringify and remark-parse #383

Merged
merged 21 commits into from Jul 20, 2019

Conversation

mike-north
Copy link
Contributor

This pull request adds typescript ambient type information for the remark-stringify package, as well as acceptance tests for types that's run in CI as a second Travis-CI stage

This change enables things TypeScript consumers to do things like this

import * as remarkStringify from 'remark-stringify'
/*
 * 👇 "ghost module" containing only type information, to avoid risk of colliding
 * w/ anything in a value-exporting module 
 */
import {RemarkStringifyOptions} from 'remark-stringify/types'
import * as unified from 'unified'

/** 
 * 👇 type-checking on the options object, including string literal types
 * for things like `fence`, `bullet`, etc...
 */
const stringifyOptions: Partial<RemarkStringifyOptions> = {
  gfm: true,
  bullet: '*',
  fences: true,
  fence: '`',
  incrementListMarker: false
}

unified().use(remarkStringify, stringifyOptions)

Copy link
Member

@Rokt33r Rokt33r left a comment

Choose a reason for hiding this comment

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

Check the prior works in unified ecosystem. vfile/vfile#33

And please update https://github.com/remarkjs/remark/blob/master/package.json too.

  • Update format script for prettier to format .ts files also.

packages/remark-cli/cli.js . -qfo && prettier --write '**/*.{js,ts}' && xo --fix

  • Add test-types script to test types of each package via lerna.
  • Append npm run test-types to test script.

BTW, thanks for the contribution! 😄

.travis.yml Outdated Show resolved Hide resolved
packages/remark-stringify/package.json Outdated Show resolved Hide resolved
packages/remark-stringify/package.json Outdated Show resolved Hide resolved
packages/remark-stringify/package.json Outdated Show resolved Hide resolved
packages/remark-stringify/types/index.d.ts Outdated Show resolved Hide resolved
packages/remark-stringify/types/index.d.ts Outdated Show resolved Hide resolved
packages/remark-stringify/types/tsconfig.json Show resolved Hide resolved
packages/remark-stringify/types/tslint.json Show resolved Hide resolved
@mike-north mike-north force-pushed the stringify-types branch 2 times, most recently from 2b45319 to 7cb0218 Compare January 15, 2019 07:21
@Rokt33r Rokt33r changed the title feat: TS type information for remark-stringify feat: TS type information for remark-stringify and remark-parse Jan 21, 2019
@Rokt33r

This comment has been minimized.

@ChristianMurphy

This comment has been minimized.

@mike-north

This comment has been minimized.

Copy link
Member

@Rokt33r Rokt33r left a comment

Choose a reason for hiding this comment

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

@wooorm I reviewed it again. It looks good to me now.

The only problem is how should we deploy this. The change will force adopters to use typescript v3.2 or above. So I think we should increase major version.

@wooorm
Copy link
Member

wooorm commented Jul 2, 2019

Perfect! Yes, remark is about to be updated with a major version, so that’s fine!

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.

Does remark itself need types?

wooorm pushed a commit to unifiedjs/unified that referenced this pull request Jul 11, 2019
Related to remarkjs/remark#383.
Closes GH-58.

Reviewed-by: Junyoung Choi <fluke8259@gmail.com>
Reviewed-by: Titus Wormer <tituswormer@gmail.com>
wooorm pushed a commit to unifiedjs/unified that referenced this pull request Jul 12, 2019
Related to remarkjs/remark#383.
Closes GH-59.

Reviewed-by: Junyoung Choi <fluke8259@gmail.com>
Reviewed-by: Titus Wormer <tituswormer@gmail.com>
@ChristianMurphy
Copy link
Member

Does remark itself need types?

@wooorm good call, remark itself also has types in the latest updates to this PR

@wooorm wooorm merged commit 8d02516 into remarkjs:master Jul 20, 2019
@wooorm wooorm added the 🧑 semver/major This is a change label Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑 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

4 participants