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

Renew API style #1

Open
Rokt33r opened this issue Sep 23, 2017 · 3 comments
Open

Renew API style #1

Rokt33r opened this issue Sep 23, 2017 · 3 comments

Comments

@Rokt33r
Copy link
Owner

Rokt33r commented Sep 23, 2017

Overloading

Lots of APIs are using overloading. (ex: Unified#use and Unified#data) I think overloading is quite harmful. To use this, we need to implement lots of assertion code and those assertions seem to be quite fragile.

I'm going to split those APIs

2 types of Parser and Compiler

A function or a class can be Parser or Compiler. To make this possible, we have to use another fragile assertion, checking the parser or the compiler is a function or not.

I think it should be better to use a class for all times.

data property in Unified

I think data is quite useless. To share the options, we can provide them to each plugin.

@Rokt33r
Copy link
Owner Author

Rokt33r commented Sep 24, 2017

ParserInstance Interface

The first argument, doc, isn't used at all and it looks redundant.

https://github.com/wooorm/remark/blob/master/packages/remark-parse/lib/parser.js#L12

@Rokt33r
Copy link
Owner Author

Rokt33r commented Sep 24, 2017

Confusing var name(Position and Point)

Although there are definitions, some libraries are using different name, like Location for Position and Position for Point.

Suggestion:

  • A specific point : Position
  • A range with 2 points : Range

@Rokt33r
Copy link
Owner Author

Rokt33r commented Oct 4, 2017

Node with Props

To create a typing for a Node with props, using type NodeWithProps<P> = Node & P is considered fragile.
Just like React component, it should be better to provide a dedicated property, props, to use custom fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant