Skip to content
This repository was archived by the owner on Nov 18, 2024. It is now read-only.

Configure TypeScript #30

Merged
merged 4 commits into from
Jan 16, 2019
Merged

Configure TypeScript #30

merged 4 commits into from
Jan 16, 2019

Conversation

willdurand
Copy link
Member

Fixes #23


This PR adds TypeScript as recommended by the CRA documentation.

I also fixed the file permissions because CRA did not set the right permissions when the project has been created.

@kumar303
Copy link
Contributor

Well, huh, this error doesn't read as well as Flow's. It gets the point across, though.

/Users/kumar/dev/addons-code-manager/src/index.tsx
Type error: Type '{ foo: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<App> & Readonly<{ children?: ReactNode; }> & Readonly<{}>'.
  Property 'foo' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<App> & Readonly<{ children?: ReactNode; }> & Readonly<{}>'.  TS2322

     5 | import * as serviceWorker from './serviceWorker';
     6 | 
  >  7 | ReactDOM.render(<App foo="wrong" />, document.getElementById('root') as HTMLElement);
       |                  ^
     8 | 
     9 | // If you want your app to work offline and load faster, you can change
    10 | // unregister() to register() below. Note this comes with some pitfalls.

@willdurand
Copy link
Member Author

I rebased this branch to fix the conflicts.

@willdurand
Copy link
Member Author

Well, huh, this error doesn't read as well as Flow's. It gets the point across, though.

Today I learnt that TS apparently distinguishes public and internal props automatically. I am not sure if that is 100% correct/true or even reliable, but that looks very promising!

Also, the TS core team is more and more focused on developer experience so I expect better compiler messages in the near future.

@kumar303
Copy link
Contributor

I can't get a typescript error in a test file to show up anywhere. Can you?

diff --git a/src/App.test.tsx b/src/App.test.tsx
index a754b20..3a04aad 100644
--- a/src/App.test.tsx
+++ b/src/App.test.tsx
@@ -4,6 +4,6 @@ import App from './App';
 
 it('renders without crashing', () => {
   const div = document.createElement('div');
-  ReactDOM.render(<App />, div);
+  ReactDOM.render(<App foo="wrong" />, div);
   ReactDOM.unmountComponentAtNode(div);
 });

We definitely want to see type errors in test files.

@willdurand
Copy link
Member Author

I can't get a typescript error in a test file to show up anywhere. Can you?

Yep, I have that in my vim. Just like Jest does not output Flow errors, we don't have TS errors. I was expecting TS to abort the compilation though... It's not that smart or maybe there is a super strict mode.

@kumar303
Copy link
Contributor

Can you share your vim config for that?

We need to, at the very least, make the build fail when there is a type error in a test file. If it's hard, we could defer it to a future issue.

@willdurand
Copy link
Member Author

willdurand commented Jan 16, 2019

Can you share your vim config for that?

let me push that

We need to, at the very least, make the build fail when there is a type error in a test file. If it's hard, we could defer it to a future issue.

Yep, this PR brings basic TS support. Let me open an issue to fail the build on TS errors, just like we have for Flow. ===> #33

Copy link
Contributor

@kumar303 kumar303 left a comment

Choose a reason for hiding this comment

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

This looks like a good start

@kumar303
Copy link
Contributor

I have some ideas for #33 so I'll merge this to get started on it.

@kumar303 kumar303 merged commit 70cb02e into master Jan 16, 2019
@willdurand willdurand deleted the typescript branch January 16, 2019 19:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure TypeScript in Create React App
2 participants