-
Notifications
You must be signed in to change notification settings - Fork 70
refactor: migrate user-agent packages to TS #1393
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
Changes from 4 commits
ee018f1
7168b08
08b6375
d7af22e
7eef67d
8b8effd
b9475c2
e653b0d
12e25dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,3 +10,4 @@ scripts/* | |
| **/scripts/* | ||
| node_modules/* | ||
| **/node_modules/* | ||
| **/typescript-sdk/* | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,14 @@ | ||
| { | ||
| "semi": false, | ||
| "trailingComma": "es5", | ||
| "singleQuote": true | ||
| } | ||
| "singleQuote": true, | ||
| "parser": "babel-flow", | ||
| "overrides": [ | ||
| { | ||
| "files": "*.ts", | ||
| "options": { | ||
| "parser": "typescript" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| module.exports = { | ||
| runner: 'jest-runner-eslint', | ||
| displayName: 'eslint', | ||
| testMatch: ['<rootDir>/packages/**/*.js', '<rootDir>/packages/**/*.ts'], | ||
| modulePathIgnorePatterns: ['dist', 'lib'], | ||
| testMatch: ['<rootDir>/**/*.js', '<rootDir>/**/*.ts'], | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| { | ||
| "lerna": "3.15.0", | ||
| "lerna": "3.16.4", | ||
| "version": "independent", | ||
| "useWorkspaces": true, | ||
| "npmclient": "yarn", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,7 +108,7 @@ export default class CsvParserState { | |
| transitions, | ||
| ...remainingState | ||
| }: StateWithUnresolvedTransitions): Promise<SuccessResult> { | ||
| return new Promise((resolve: Function) => { | ||
| return new Promise((resolve: Function, reject: Function) => { | ||
| if (!transitions) resolve(remainingState) | ||
| // We setup the client here because it is unnecessary | ||
| // if there are no transitions | ||
|
|
@@ -125,17 +125,19 @@ export default class CsvParserState { | |
| ) | ||
| ) | ||
| ) | ||
| Promise.all(stateRequests).then( | ||
| (resolvedStates: Array<SuccessResult>) => { | ||
| Promise.all(stateRequests) | ||
| .then((resolvedStates: Array<SuccessResult>) => { | ||
| const stateReferences = resolvedStates.map( | ||
| (response: SuccessResult): StateReference => ({ | ||
| typeId: 'state', | ||
| id: response.body.id, | ||
| }) | ||
| ) | ||
| resolve({ ...remainingState, transitions: stateReferences }) | ||
| } | ||
| ) | ||
| }) | ||
| .catch((error: Error) => { | ||
| reject(error) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was missing. Somehow the tests caught this now |
||
| }) | ||
| }) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,12 +125,14 @@ describe('CsvParserState', () => { | |
|
|
||
| describe('::_transformTransitions', () => { | ||
| describe('without state transitions', () => { | ||
| test('should not mutate state', async () => { | ||
| test('should not mutate state', () => { | ||
| const actual = { | ||
| foo: 'bar', | ||
| key: 'my-state', | ||
| } | ||
| expect(csvParser._transformTransitions(actual)).resolves.toEqual(actual) | ||
| return expect(csvParser._transformTransitions(actual)).resolves.toEqual( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There were a bunch of tests that did not return the expectation with the promise. |
||
| actual | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
|
|
@@ -159,11 +161,10 @@ describe('CsvParserState', () => { | |
| ) | ||
| }) | ||
|
|
||
| test('should resolve with modified state', () => { | ||
| test('should resolve with modified state', () => | ||
| expect( | ||
| csvParser._transformTransitions(state) | ||
| ).resolves.toMatchSnapshot() | ||
| }) | ||
| ).resolves.toMatchSnapshot()) | ||
| }) | ||
|
|
||
| describe('when execute rejects', () => { | ||
|
|
@@ -173,11 +174,10 @@ describe('CsvParserState', () => { | |
| ) | ||
| }) | ||
|
|
||
| test('should reject with execute rejection error', () => { | ||
| test('should reject with execute rejection error', () => | ||
| expect(csvParser._transformTransitions(state)).rejects.toThrow( | ||
| /Invalid Request/ | ||
| ) | ||
| }) | ||
| )) | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export default from './main' | ||
| export { default } from './main' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| module.exports = { | ||
| parser: '@typescript-eslint/parser', | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned before, for now TS packages should have their own eslint config, which is merged with the base one. Once we've migrated all packages, we can merge this back with the base one. |
||
| extends: [ | ||
| 'plugin:@typescript-eslint/recommended', | ||
| 'plugin:import/typescript', | ||
| 'plugin:jest/recommended', | ||
| 'prettier', | ||
| 'prettier/@typescript-eslint', | ||
| ], | ||
| plugins: ['import', 'jest', 'prettier', '@typescript-eslint/eslint-plugin'], | ||
| env: { | ||
| es6: true, | ||
| browser: true, | ||
| jest: true, | ||
| node: true, | ||
| }, | ||
| rules: { | ||
| '@typescript-eslint/camelcase': 0, | ||
| '@typescript-eslint/consistent-type-definitions': 0, | ||
| '@typescript-eslint/explicit-function-return-type': 0, | ||
| '@typescript-eslint/no-explicit-any': 2, | ||
| '@typescript-eslint/no-use-before-define': ['error', { functions: false }], | ||
| '@typescript-eslint/no-var-requires': 0, | ||
| '@typescript-eslint/unbound-method': 0, | ||
| 'import/no-extraneous-dependencies': 0, | ||
| 'import/no-named-as-default': 0, | ||
| 'import/no-unresolved': 2, | ||
| }, | ||
| settings: { | ||
| 'import/parsers': { | ||
| '@typescript-eslint/parser': ['.ts'], | ||
| }, | ||
| 'import/resolver': { | ||
| 'eslint-import-resolver-typescript': true, | ||
| typescript: {}, | ||
| node: { | ||
| extensions: ['.js', '.ts'], | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
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.
Here we use the
overridefeature to tell prettier to usetypescriptparser for the ts files, otherwise thebabel-flowparser.