iLoveElysia/pjax
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 9 Wiki Activity

Add Typescript definitions #138

Merged
BehindTheMath merged 8 commits from feature/ts-defs into master 2018-03-15 20:16:11 -05:00
Conversation 6 Commits 8 Files Changed 4
+236 -4
BehindTheMath commented 2018-03-10 23:37:10 -05:00 (Migrated from github.com)
Copy link
Copy source
No description provided.
BehindTheMath commented 2018-03-10 23:42:43 -05:00 (Migrated from github.com)
Copy link
Copy source

Never mind.

According to Typescript's documentation, if the package is not written in Typescript, it's preferable to add the type definitions to the DefinitelyTyped repo.

Never mind. According to [Typescript's documentation](https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html), if the package is not written in Typescript, it's preferable to add the type definitions to the DefinitelyTyped repo.
BehindTheMath commented 2018-03-14 14:42:54 -05:00 (Migrated from github.com)
Copy link
Copy source

Actually, DefinitelyTyped's recommendation is:

If you are the library author, or can make a pull request to the library, bundle types instead of publishing to DefinitelyTyped.

So I'm reopening this PR to add the definitions here.

Actually, [DefinitelyTyped's recommendation](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/63ecc4d05dbd2571cc5533a9060823078cc72643#create-a-new-package) is: >If you are the library author, or can make a pull request to the library, bundle types instead of publishing to DefinitelyTyped. So I'm reopening this PR to add the definitions here.
robinnorth commented 2018-03-14 15:18:00 -05:00 (Migrated from github.com)
Copy link
Copy source

LGTM, though I'm not a Typescript user. You might want to add a note to the readme to the effect that any PRs should ideally include updated Typescript definitions if necessary, in addition to passing tests.

LGTM, though I'm not a Typescript user. You might want to add a note to the readme to the effect that any PRs should ideally include updated Typescript definitions if necessary, in addition to passing tests.
BehindTheMath commented 2018-03-14 15:45:38 -05:00 (Migrated from github.com)
Copy link
Copy source

I'm going to try to figure out if there's an easy way to write tests for this. Then we could include that in the testing workflow.

I'm going to try to figure out if there's an easy way to write tests for this. Then we could include that in the testing workflow.
robinnorth (Migrated from github.com) approved these changes 2018-03-15 04:35:07 -05:00
robinnorth (Migrated from github.com) left a comment
Copy link
Copy source

Great, sounds good to me. Feel free to merge when you're happy!

Great, sounds good to me. Feel free to merge when you're happy!
BehindTheMath commented 2018-03-15 15:16:57 -05:00 (Migrated from github.com)
Copy link
Copy source

I haven't added any tooling for actually running or automating the tests. To run it, Typescript would need to be installed, and I don't want to deal with that now.

To run it manually, install Typescript (either locally or globally), and then run:

tsc --noEmit ./tests/test.ts
I haven't added any tooling for actually running or automating the tests. To run it, Typescript would need to be installed, and I don't want to deal with that now. To run it manually, install Typescript (either locally or globally), and then run: ``` tsc --noEmit ./tests/test.ts ```
Sign in to join this conversation.
Reviewers
No Reviewers
robinnorth
Labels
Clear labels
bug
dependencies
Pull requests that update a dependency file
 duplicate
enhancement
invalid
question
wontfix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
iLoveElysia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#138