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

Add Typescript (and bypass Babel) #658

Merged
merged 52 commits into from
Feb 2, 2018
Merged

Add Typescript (and bypass Babel) #658

merged 52 commits into from
Feb 2, 2018

Conversation

seanf
Copy link
Contributor

@seanf seanf commented Jan 12, 2018

@seanf
Copy link
Contributor Author

seanf commented Jan 12, 2018

Unit tests are passing, but currently having problems resolving some modules in the browser when loading the root web page. Both with frontend's make watch (http://localhost:8000)

frontend.css Failed to load resource: the server responded with a status of 404 (Not Found)
webpack:///./app/utils/DateHelper.js?:101 Uncaught TypeError: moment_1.default is not a function
    at Object.startDate (webpack:///./app/utils/DateHelper.js?:101)
    at Object.getDateRange (webpack:///./app/utils/DateHelper.js?:144)
    at eval (webpack:///./app/reducers/profile-reducer.js?:204)
    at Object.<anonymous> (frontend.js:17057)
    at __webpack_require__ (frontend.js:20)
    at eval (webpack:///./app/reducers/index.js?:8)
    at Object.<anonymous> (frontend.js:16145)
    at __webpack_require__ (frontend.js:20)
    at eval (webpack:///./app/index.js?:14)
    at Object.<anonymous> (frontend.js:11337)
frontend.css Failed to load resource: the server responded with a status of 404 (Not Found)

and proper war deployment (apparently having trouble with import PropTypes from 'prop-types'):

NavIcon.jsx:31 Uncaught TypeError: Cannot read property 'string' of undefined
    at eval (NavIcon.jsx:31)
    at Object.08gb (runtime.69fb9ed8.cache.js:441)
    at __webpack_require__ (runtime.69fb9ed8.cache.js:55)
    at eval (NavItem.jsx:22)
    at Object.HPNb (runtime.69fb9ed8.cache.js:2797)
    at __webpack_require__ (runtime.69fb9ed8.cache.js:55)
    at eval (index.jsx:22)
    at Object.3pQB (runtime.69fb9ed8.cache.js:869)
    at __webpack_require__ (runtime.69fb9ed8.cache.js:55)
    at eval (index.js:3)

EDIT: I was fixing problems like these as I found them by changing imports (eg const xyz = require('xyz'); const abc = require('abc').default), but now TypeScript 2.7 is going to behave like babel for ES module interop so TypeScript 2.7 plus --esModuleInterop solved the problem.

@codecov
Copy link

codecov bot commented Jan 12, 2018

Codecov Report

Merging #658 into master will increase coverage by 0.27%.
The diff coverage is 39.6%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #658      +/-   ##
============================================
+ Coverage     33.15%   33.43%   +0.27%     
  Complexity     5778     5778              
============================================
  Files          1577     1580       +3     
  Lines         62287    63216     +929     
  Branches       7354     7365      +11     
============================================
+ Hits          20654    21136     +482     
- Misses        39701    40145     +444     
- Partials       1932     1935       +3
Impacted Files Coverage Δ Complexity Δ
...r/zanata-frontend/src/app/editor/reducers/index.js 100% <ø> (ø) 0 <0> (ø) ⬇️
...r/zanata-frontend/src/app/components/Link/index.js 100% <ø> (ø) 0 <0> (ø) ⬇️
...a-frontend/src/app/components/Modal/ModalTitle.jsx 100% <ø> (ø) 0 <0> (ø) ⬇️
.../src/app/containers/ProjectVersion/TMMergeModal.js 0% <ø> (ø) 0 <0> (ø) ⬇️
...ta-frontend/src/app/components/LoaderText/index.js 100% <ø> (ø) 0 <0> (ø) ⬇️
server/zanata-frontend/src/app/jsf/index.js 0% <ø> (ø) 0 <0> (ø) ⬇️
...ontend/src/app/editor/components/TextDiff/index.js 90.9% <ø> (+7.57%) 0 <0> (ø) ⬇️
.../zanata-frontend/src/app/actions/common-actions.js 91.3% <ø> (ø) 0 <0> (ø) ⬇️
...ta-frontend/src/app/components/Modal/ModalBody.jsx 100% <ø> (ø) 0 <0> (ø) ⬇️
...-frontend/src/app/components/Modal/ModalHeader.jsx 100% <ø> (ø) 0 <0> (ø) ⬇️
... and 284 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8fe452...5cdc513. Read the comment docs.

@seanf seanf force-pushed the typescript branch 3 times, most recently from 10c3429 to dd27072 Compare January 19, 2018 14:02
@seanf seanf changed the title Use Typescript instead of Babel Add Typescript (and bypass Babel) Jan 31, 2018
@huangp
Copy link
Contributor

huangp commented Feb 1, 2018

Reviewed 2 of 8 files at r1, 1 of 3 files at r2, 24 of 248 files at r3, 14 of 34 files at r4, 28 of 35 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks broke.


server/zanata-frontend/src/tsconfig.json, line 8 at r5 (raw file):

        "useCache": true
    },
    "compilerOptions": {

a link to the doc will be helpful


server/zanata-frontend/src/app/components/Loader/index.tsx, line 16 at r5 (raw file):

interface LoaderProps {
  className: string

This should be optional


server/zanata-frontend/src/app/components/LockIcon/index.tsx, line 24 at r5 (raw file):

interface LockIconProps {
  status: any // TODO should be one of entityStatuses

make entityStatus a ts file and add types


server/zanata-frontend/src/app/editor/actions/settings-actions.js, line 33 at r5 (raw file):

// Should be pretty fast: https://stackoverflow.com/a/34491287/14379
function emptyObject (obj) {

move this to util


server/zanata-frontend/src/scripts/silence-React-15-deprecations.js, line 1 at r5 (raw file):

const React = require('react')

this file is not needed


Comments from Reviewable

@seanf
Copy link
Contributor Author

seanf commented Feb 1, 2018

Review status: 51 of 71 files reviewed at latest revision, 5 unresolved discussions.


server/zanata-frontend/src/tsconfig.json, line 8 at r5 (raw file):

Previously, huangp (Patrick Huang) wrote…

a link to the doc will be helpful

Done.


server/zanata-frontend/src/app/components/Loader/index.tsx, line 16 at r5 (raw file):

Previously, huangp (Patrick Huang) wrote…

This should be optional

Done.


server/zanata-frontend/src/app/components/LockIcon/index.tsx, line 24 at r5 (raw file):

Previously, huangp (Patrick Huang) wrote…

make entityStatus a ts file and add types

Done.


server/zanata-frontend/src/app/editor/actions/settings-actions.js, line 33 at r5 (raw file):

Previously, huangp (Patrick Huang) wrote…

move this to util

Done.


server/zanata-frontend/src/scripts/silence-React-15-deprecations.js, line 1 at r5 (raw file):

Previously, huangp (Patrick Huang) wrote…

this file is not needed

Done.


Comments from Reviewable

@seanf
Copy link
Contributor Author

seanf commented Feb 1, 2018

@huangp Ready for another review. Thanks!

@huangp
Copy link
Contributor

huangp commented Feb 1, 2018

:lgtm: +Reviewed


Reviewed 5 of 36 files at r5, 18 of 23 files at r6, 14 of 14 files at r7.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@huangp huangp added the Reviewed label Feb 1, 2018
@huangp
Copy link
Contributor

huangp commented Feb 2, 2018

Reviewed 1 of 23 files at r6, 5 of 5 files at r8.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@djansen-redhat
Copy link
Contributor

Verified (exploratory testing)

@seanf seanf merged commit 5bd626d into master Feb 2, 2018
@seanf seanf deleted the typescript branch February 2, 2018 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants