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

Feature/issues 18 typescript support #19

Merged
merged 8 commits into from
Mar 19, 2020

Conversation

alewin
Copy link
Owner

@alewin alewin commented Mar 16, 2020

Todo

  • Write comments
  • Add a draft on typescript type
  • Define types
  • Testing
  • Add example
  • Review

@alewin alewin linked an issue Mar 17, 2020 that may be closed by this pull request
Copy link
Collaborator

@Pigotz Pigotz left a comment

Choose a reason for hiding this comment

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

The overall PR looks good, I just pointed some issues for further improvements.

Also, the JSdoc comments are useful but, to me, out of the scope for this PR.
Next steps could possibly be:

  • Move all of the source code to TS
  • Generate a real JSdoc out of those comments

package.json Show resolved Hide resolved
Comment on lines -9 to -11
"build": "microbundle src/index.js && npm run copy",
"build": "microbundle src/index.js",
"dev": "microbundle watch",
"copy": "cp package.json dist/ && cp README.md dist/",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would restore this and make two changes:

  • Use a cross platform copy command (this won't work on Windows) like cpx
  • Copy the index.d.ts file too

Copy link
Owner Author

Choose a reason for hiding this comment

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

but would that be redundant? I already keep these files inside the root

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds ok for the index.d.ts file. What about the cpx package?

index.d.ts Outdated Show resolved Hide resolved
@alewin alewin merged commit 5fa203c into develop Mar 19, 2020
@alewin alewin deleted the feature/issues-18-typescript-support branch March 22, 2020 11:32
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.

Consider a TypeScript support
2 participants