-
Notifications
You must be signed in to change notification settings - Fork 142
Add TypeScript definitions to enforce or infer whether properties are required. #114
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
62960e2
Add optional generic constraint for FSA type property
fredrikhr 1c2f206
Get jest to also test TypeScript tests
fredrikhr fbdba4b
Optional Payload type constraint
fredrikhr ec2abeb
Add comments to FSA types and properties
fredrikhr b291523
Add FSA extensions with required properties
fredrikhr 2ba3a81
Simplified FSA TypeScript test
JaKXz ddbdf6c
fixup! Simplified FSA TypeScript test
fredrikhr fd7f351
Add test for FSAAuto type
fredrikhr 6f9d298
Update Jest and ESLint dev dependencies
fredrikhr b73243f
Move generic argument Type to the front for all FSA types
fredrikhr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
module.exports = { | ||
transform: { | ||
'^.+\\.js$': 'babel-jest', | ||
'^.+\\.tsx?$': 'ts-jest' | ||
}, | ||
testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.[jt]sx?$', | ||
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'] | ||
}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import { FSAAuto, isFSA } from '../src'; | ||
|
||
describe('Usage of FSAAuto (automatically infer required properties', () => { | ||
it('must specify payload property even when using a union with undefined', () => { | ||
const fsa_with_payload = { type: 'TEST', payload: undefined }; | ||
expectOptionalPayload(fsa_with_payload); | ||
|
||
const fsa_without_payload = { type: 'TEST' }; | ||
// Not possible to cast!!! | ||
// expectOptionalPayload(fsa_without_payload); | ||
|
||
function expectOptionalPayload(fsa: FSAAuto<string | undefined>) { | ||
expect(fsa.payload).toBeUndefined(); | ||
} | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import { FSA } from '../src'; | ||
|
||
const ACTION_TYPE_1 = 'ACTION_TYPE_1'; | ||
type ACTION_TYPE_1 = typeof ACTION_TYPE_1; | ||
type FSA_ACTION_TYPE_1 = FSA<undefined, undefined, ACTION_TYPE_1>; | ||
|
||
const assertNever = (x: never): never => { | ||
throw new Error(`Unexpected value: ${x}.`); | ||
}; | ||
|
||
const assertTypeValue = (fsa: FSA_ACTION_TYPE_1) => { | ||
expect(fsa.type).toBe(ACTION_TYPE_1); | ||
}; | ||
|
||
describe('FluxStandardAction<Payload, Meta, Type>', () => { | ||
it('enables TypeScript action type enforcement', () => { | ||
const fsa_strict: FSA_ACTION_TYPE_1 = { type: ACTION_TYPE_1 }; | ||
assertTypeValue(fsa_strict); | ||
if (fsa_strict.type !== ACTION_TYPE_1) { | ||
fredrikhr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw assertNever(fsa_strict.type); | ||
} | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,5 @@ | |
"strictNullChecks": true, | ||
"target": "es5" | ||
}, | ||
"files": [ | ||
"test/typings.test.ts" | ||
] | ||
"files": ["src/index.d.ts"] | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what the benefit of creating
Type
and using theextends
keyword is, as opposed to just saying it must be astring
. I'm not up to speed on the latest TS trends 😅There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In TypeScript you can restrict a type to a constant value.
For a Counter application example this makes it possible to change the following code:
to this:
removing the need to re-declare the
type
property. An object using a string, but different to 'INCREMENT_COUNTER' would thus not implement theIncrementCounterAction
type.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@couven92 Why would we want to put the
Type
generic to be last? This would mean we would always have to explicitly type meta whether the action has meta or not.I think I would prefer to put
tType
first as it is the primary property of an FSA and I want to always strongly type my action types for discriminitive unions in redux reducers. Where as quite a lot of my action don't utilise theMeta
property.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My opinionated take on this is on the master branch of this fork if you want to have a look https://github.com/hally9k/flux-standard-action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #114 (comment)