Convert core components to TypeScript (Round 2) #9

trblackw · 2019-06-01T21:10:18Z

@nickmcmillan @vaibhavhrt Okay, everything now should be able to be merged smoothly considering index.js (now index.tsx) is hook-based. I apologize for the merge conflict hiccup.

nickmcmillan · 2019-06-03T17:02:22Z

@trblackw thanks for your work on this (and no stress about the merge conflict)!

However I'm having a little trouble getting this branch to run. I'm seeing the message ../dist/index.es.js Module not found: Can't resolve './styles.css'.

I've run yarn to ensure all dependencies are updated. Is there something else I'm missing?

trblackw · 2019-06-03T17:06:22Z

Yeah I attempted to address that my adding the resolve plugin to the roll up config. TypeScript doesn’t like the import statement when importing files that aren’t .ts/.tsx/.js. The require syntax that I used at the top of the refactored index.tsx combined with the plugin added to the roll up config should have fixed this 🧐 I can look into it

…

On Mon, Jun 3, 2019 at 1:02 PM Nick McMillan ***@***.***> wrote: @trblackw <https://github.com/trblackw> thanks for your work on this (and no stress about the merge conflict)! However I'm having a little trouble getting this branch to run. I'm seeing the message ../dist/index.es.js Module not found: Can't resolve './styles.css'. I've run yarn to ensure all dependencies are updated. Is there something else I'm missing? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT26UVGMPBOB3K42Q3KDPYVFB7A5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW2BNYY#issuecomment-498341603>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT22IMZNAZM3EG2ZLOWTPYVFB7ANCNFSM4HSBJV2Q> .

trblackw · 2019-06-03T17:09:08Z

This might help https://link.medium.com/wE7MP8SVdX I’m away from my computer for the next day so (vacation from work) if you could maybe find a solution before then maybe that would be easier? Otherwise I can address it in the next couple days

…

On Mon, Jun 3, 2019 at 1:06 PM Tucker Blackwell ***@***.***> wrote: Yeah I attempted to address that my adding the resolve plugin to the roll up config. TypeScript doesn’t like the import statement when importing files that aren’t .ts/.tsx/.js. The require syntax that I used at the top of the refactored index.tsx combined with the plugin added to the roll up config should have fixed this 🧐 I can look into it On Mon, Jun 3, 2019 at 1:02 PM Nick McMillan ***@***.***> wrote: > @trblackw <https://github.com/trblackw> thanks for your work on this > (and no stress about the merge conflict)! > > However I'm having a little trouble getting this branch to run. I'm > seeing the message ../dist/index.es.js Module not found: Can't resolve > './styles.css'. > > I've run yarn to ensure all dependencies are updated. Is there something > else I'm missing? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#9?email_source=notifications&email_token=AIVCT26UVGMPBOB3K42Q3KDPYVFB7A5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW2BNYY#issuecomment-498341603>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AIVCT22IMZNAZM3EG2ZLOWTPYVFB7ANCNFSM4HSBJV2Q> > . >

vaibhavhrt · 2019-06-12T04:55:46Z

@trblackw waiting for you to complete this before I can start working on #11

trblackw · 2019-06-12T10:01:51Z

I can’t seem to replicate the issue you were having on my end. When are you seeing the error message you listed above? When running `yarn start`? I just pushed a change to include `.css` extensions in the path options within the rollup config. Let me know if this resolves the issue on your end.

…

On Wed, Jun 12, 2019 at 12:55 AM Vaibhav Vishal ***@***.***> wrote: @trblackw <https://github.com/trblackw> waiting for you to complete this before I can start working on #11 <#11> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT27LN7JLYR2QUSMR7U3P2B6VFA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXPH2LQ#issuecomment-501120302>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT25LE2TNARVLKUJM72LP2B6VFANCNFSM4HSBJV2Q> .

…tion

vaibhavhrt · 2019-06-12T10:39:43Z

@trblackw you need to merge upstream/master

trblackw · 2019-06-12T10:49:11Z

@nickmcmillan Forgive me because I'm not very familiar with working with forked copies of repos. I'm currently on master of my fork and getting this error when attempting to merge:

I'm assuming there's some precursor steps I need to follow in order to be able to execute that command successfully?

edit: I did however just git pull [email protected]:nickmcmillan/react-physics-dragger.git and then resolve any merge conflicts and push.

vaibhavhrt · 2019-06-12T10:51:59Z

Make sure you are on branch master:

git checkout master

Fetch remote:

git fetch

Merge remote:

git merge upstream/master

trblackw · 2019-06-12T10:54:38Z

@vaibhavhrt What's weird is git fetch did not result in the fetching of the origin branch, which is why running git merge upstream/master was resulting in the screen shot I added above, I presume. I'm a bit of a git noob as well so I probably messed something up along the way. I see the conflicts have been resolved after what I just pushed, does that mean we're good to go?

I am noticing that Travis CI isn't very happy though

These environment variables weren't something I messed with on my end

vaibhavhrt · 2019-06-12T11:23:13Z

In conflict resolving you probably forced your changes instead of accepting incoming changes so eslint is giving errors.
Go src/package.json, find the lintJs commands in scripts section and add fix option to it:

"lintJs": "eslint **/*.{js,jsx} --fix"

Run it: npm run lintJs, it will fix travis errors.
undo changes in package.json, so it should be "lintJs": "eslint **/*.{js,jsx}" again.
Also make sure you have deleted src/index.js

trblackw · 2019-06-12T11:26:08Z

Okay will have this done by 12:00 EST. Apologies for making this more difficult than it should be.

…

On Wed, Jun 12, 2019 at 7:23 AM Vaibhav Vishal ***@***.***> wrote: In conflict resolving you probably forced your changes instead of accepting incoming changes so eslint is giving errors. Go package.json, find the lintJs commands in scripts section and add fix option to it: "lintJs": "eslint **/*.{js,jsx} --fix" Run it: npm run lint, it fix fix travis errors. undo changes in package.json, so it should be "lintJs": "eslint **/*.{js,jsx}" again. Also make sure you have deleted src/index.js — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT2Y2MBKKALA7DGCGWBLP2DMCFA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQC2WA#issuecomment-501230936>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT2YAI4B4EW765KAYM23P2DMCFANCNFSM4HSBJV2Q> .

vaibhavhrt · 2019-06-12T11:39:33Z

Actually forget my last comment, you actually messed up eslint settings, nothing wrong in resolving conflicts. We were using 2 spaces for tabs, you changed it 4 and trailing comma was allowed, you removed it too. See my review for fixing it.

trblackw · 2019-06-12T11:40:51Z

👍🏼

…

On Wed, Jun 12, 2019 at 7:39 AM Vaibhav Vishal ***@***.***> wrote: Actually forget my last comment, you actually messed up eslint settings, nothing wrong in resolving conflicts. We were using 2 spaces for tabs, you changed it 4 and trailing comma was allowed, you removed it too. See my review for fixing it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT22PL5PXGCD36XXCQNTP2DN7LA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQD7PQ#issuecomment-501235646>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT23TC5HHFXG7645MRVDP2DN7LANCNFSM4HSBJV2Q> .

vaibhavhrt

Fix these two 👇 , run npm run lintJs to make sure there are no more errors, if there is fix them.

vaibhavhrt · 2019-06-12T11:40:21Z

.eslintrc

-    "react/jsx-boolean-value": 0,
-
-    // allow comma-dangle in multiline objects & arrays
-    "comma-dangle": ["error", "only-multiline"]


This line needs to be there, add it back.

"comma-dangle": ["error", "only-multiline"]

vaibhavhrt · 2019-06-12T11:42:15Z

.eslintrc

+   "extends": [
+     "standard",
+     "standard-react",
+     "plugin:@typescript-eslint/recommended"


Extending this rule is probably forcing 4 spaces for indent. Add this line in rules section to make tab size 2 spaces:

"indent": ["error", 2],

See https://stackoverflow.com/a/48906878/9321755 for more details.

trblackw · 2019-06-12T11:52:29Z

Okay I won’t be able to push until 11:00 EST so I’ll let you know when I do

…

On Wed, Jun 12, 2019 at 7:43 AM Vaibhav Vishal ***@***.***> wrote: ***@***.**** requested changes on this pull request. Fix these 2, run npm run lintJs to make sure there are no more errors, if there is fix them. ------------------------------ In .eslintrc <#9 (comment)> : > - }, - "plugins": [ - "react" - ], - "parserOptions": { - "sourceType": "module" - }, - "rules": { - // don't force es6 functions to include space before paren - "space-before-function-paren": 0, - - // allow specifying true explicitly for boolean props - "react/jsx-boolean-value": 0, - - // allow comma-dangle in multiline objects & arrays - "comma-dangle": ["error", "only-multiline"] This line needs to be there in add it back. "comma-dangle": ["error", "only-multiline"] ------------------------------ In .eslintrc <#9 (comment)> : > - "rules": { - // don't force es6 functions to include space before paren - "space-before-function-paren": 0, - - // allow specifying true explicitly for boolean props - "react/jsx-boolean-value": 0, - - // allow comma-dangle in multiline objects & arrays - "comma-dangle": ["error", "only-multiline"] - } -} + "parser": ***@***.***/parser", + "extends": [ + "standard", + "standard-react", + ***@***.***/recommended" Extending this rule is probably forcing 4 spaces for indent. Add this line in rules section to make tab size 2 spaces: "indent": ["error", 2], See https://stackoverflow.com/a/48906878/9321755 for more details. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT25XSI6QCQ7FXU4RALLP2DOMLA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3JOJGQ#pullrequestreview-248702106>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT2Y3FWYMMYF7YZJQQ4LP2DOMLANCNFSM4HSBJV2Q> .

vaibhavhrt · 2019-06-12T11:53:22Z

K, let me know if there are still any errors On Wed, 12 Jun 2019, 5:22 pm Tucker Blackwell, <[email protected]> wrote:

…

Okay I won’t be able to push until 11:00 EST so I’ll let you know when I do On Wed, Jun 12, 2019 at 7:43 AM Vaibhav Vishal ***@***.***> wrote: > ***@***.**** requested changes on this pull request. > > Fix these 2, run npm run lintJs to make sure there are no more errors, if > there is fix them. > ------------------------------ > > In .eslintrc > < #9 (comment) > > : > > > - }, > - "plugins": [ > - "react" > - ], > - "parserOptions": { > - "sourceType": "module" > - }, > - "rules": { > - // don't force es6 functions to include space before paren > - "space-before-function-paren": 0, > - > - // allow specifying true explicitly for boolean props > - "react/jsx-boolean-value": 0, > - > - // allow comma-dangle in multiline objects & arrays > - "comma-dangle": ["error", "only-multiline"] > > This line needs to be there in add it back. > > "comma-dangle": ["error", "only-multiline"] > > ------------------------------ > > In .eslintrc > < #9 (comment) > > : > > > - "rules": { > - // don't force es6 functions to include space before paren > - "space-before-function-paren": 0, > - > - // allow specifying true explicitly for boolean props > - "react/jsx-boolean-value": 0, > - > - // allow comma-dangle in multiline objects & arrays > - "comma-dangle": ["error", "only-multiline"] > - } > -} > + "parser": ***@***.***/parser", > + "extends": [ > + "standard", > + "standard-react", > + ***@***.***/recommended" > > Extending this rule is probably forcing 4 spaces for indent. Add this line > in rules section to make tab size 2 spaces: > > "indent": ["error", 2], > > See https://stackoverflow.com/a/48906878/9321755 for more details. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #9?email_source=notifications&email_token=AIVCT25XSI6QCQ7FXU4RALLP2DOMLA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3JOJGQ#pullrequestreview-248702106 >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AIVCT2Y3FWYMMYF7YZJQQ4LP2DOMLANCNFSM4HSBJV2Q > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AFMJMLHPZNNI5I3TQKKF6WDP2DPP5A5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQE7CY#issuecomment-501239691>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFMJMLAZ4Q6YGOIKUYLOLSTP2DPP5ANCNFSM4HSBJV2Q> .

…t with eslint

trblackw · 2019-06-12T15:41:12Z

@vaibhavhrt okay I updated the eslint settings and fixed the affected files. I also just removed the tslint plugin because it was directly conflicting with eslint.

vaibhavhrt

@trblackw add rollup.config.js in .eslintignore to ignore it(or fix the errors if you want).

vaibhavhrt · 2019-06-12T15:46:54Z

src/.eslintrc

@@ -1,5 +0,0 @@
-{
-  "env": {
-    "jest": true


put this line in actual .eslintrc.

vaibhavhrt

Also add react-scripts-ts as devDependency and then update the test scripts(in package.json) to run ts tests, right now it's not finding any tests as it's looking for tests with extension js.

"test": "cross-env CI=1 react-scripts-ts test --env=jsdom",
"test:watch": "react-scripts-ts test --env=jsdom",

vaibhavhrt · 2019-06-12T15:50:35Z

package.json

+    "@types/jest": "^24.0.13",
+    "@types/react": "^16.8.19",
+    "@types/react-dom": "^16.8.4"


@types dependencies are devDependencies as well. Move them to devDependencies

trblackw · 2019-06-12T15:56:59Z

Okay. I’m offline again for a bit so I’ll address this this afternoon

…

On Wed, Jun 12, 2019 at 11:52 AM Vaibhav Vishal ***@***.***> wrote: ***@***.**** requested changes on this pull request. Also add react-scripts-ts as devDependency and then update the test scripts(in package.json) to run ts tests, right now it's not finding any tests as it's looking for tests with extension js. "test": "cross-env CI=1 react-scripts-ts test --env=jsdom", "test:watch": "react-scripts-ts test --env=jsdom", ------------------------------ In package.json <#9 (comment)> : > \ No newline at end of file + ], + "dependencies": { + ***@***.***/jest": "^24.0.13", + ***@***.***/react": "^16.8.19", + ***@***.***/react-dom": "^16.8.4" @types dependencies are devDependencies as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT27ZBTGWUXBK6Q32L2DP2ELU5A5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3KUBPI#pullrequestreview-248856765>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT247ANO5XK4BHHFI7T3P2ELU5ANCNFSM4HSBJV2Q> .

vaibhavhrt · 2019-06-12T19:49:40Z

Also add react-scripts-ts as devDependency and then update the test scripts(in package.json) to run ts tests, right now it's not finding any tests as it's looking for tests with extension js.
"test": "cross-env CI=1 react-scripts-ts test --env=jsdom",
"test:watch": "react-scripts-ts test --env=jsdom",

trblackw · 2019-06-12T19:52:25Z

Oops, forgot that one. Should be good now @vaibhavhrt

vaibhavhrt · 2019-06-13T08:38:12Z

Ok looks like ts requires a tsconfig.test.json to be able to run test suite.
Add a tsconfig.test.json in root directory with following contents.

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "module": "commonjs"
  }
}

trblackw · 2019-06-13T10:03:27Z

@vaibhavhrt done

vaibhavhrt · 2019-06-13T12:28:13Z

@trblackw test is now running which is good, but it's failing. I am not sure what's wrong, might need some investigation. Can you try to figure out what's wrong. You can run tests locally by npm run test to make sure it passes before pushing commits.
You can see error details by running tests locally or see the logs on tavis.

trblackw · 2019-06-13T12:30:35Z

I’ll take a look when I get a chance

…

On Thu, Jun 13, 2019 at 8:28 AM Vaibhav Vishal ***@***.***> wrote: @trblackw <https://github.com/trblackw> test is now running which is good, but it's failing. I am not sure what's wrong, might need some investigation. Can you try to figure out what's wrong. You can run tests locally by npm run test to make sure it passes before pushing commits. You can see error details by running tests locally or see the logs on tavis. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT225ZBK43BSYVAM3MLDP2I4N3A5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXTQZXI#issuecomment-501681373>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT262BIGV6W5CBQ3TKB3P2I4N3ANCNFSM4HSBJV2Q> .

trblackw · 2019-06-16T13:05:44Z

@vaibhavhrt So I'm been trying to trouble shoot what's going on with the test that's failing in index.tsx, but I'm not super familiar with testing (sad face). Perhaps you might have a bit more insight into what's potentially going wrong here

What's confusing to me is that the error message: Unexpected token (221:12) is actually on line 248 in my editor (perhaps this is normal because extra spaces are removed upon compilation or something?). My editor shows no visible issues with the code. The result message of the test is Test suite failed to run, which seems to contradict the previous unexpected token issue?

What do you think?

vaibhavhrt · 2019-06-16T14:26:29Z

@trblackw I will clone your fork of this repo and take a look later. rn busy watching world cup

vaibhavhrt

Problem is with tsconfig

vaibhavhrt · 2019-06-17T18:51:37Z

src/index.tsx


 import { roundNum } from './utils'
 import { applyDragForce, applyBoundForce } from './force'
 import getBoundaries from './getBoundaries'
+// TypeScript does not like using the import statement for non js/ts files
+const styles = require('./styles.css')


Actually you can import css files, you just need to create a typings.d.ts in src with following content:

/** * Default CSS definition for typescript, * will be overridden with file-specific definitions by rollup */ declare module "*.css" { const content: { [className: string]: string }; export default content; }

this didn't work for me. I'm leaving the rollup config modifications for now. test is now passing

yeah no need to actually do this. I am just telling its possible.

src/index.tsx

tsconfig.json

Co-Authored-By: Vaibhav Vishal <[email protected]>

trblackw · 2019-06-17T21:00:04Z

@vaibhavhrt Okay after using the tsconfig.json you provided above the test is passing

vaibhavhrt · 2019-06-18T08:09:23Z

Looks good to me now. @nickmcmillan you can review and merge.

nickmcmillan · 2019-06-18T11:33:34Z

I'm still unable to get the example to run in this branch. Steps to reproduce are;

Checkout this branch, run yarn to ensure dependencies are updated
yarn start in the root folder
cd example and yarn start to launch the development example

At this point I see the error message I'd mentioned here; #9 (comment) - which appears both in the terminal and in the browser.

I implemented the solution @vaibhavhrt suggested here; Convert core components to TypeScript (Round 2) #9 (comment) which worked - I'm no longer seeing the error message.

In the browser, I'm seeing a new error message (screenshot attached) which suggests to me that Typescript is not being compiled, the browser is attempting to read TS directly.

Am I missing something? Have you both tried testing the project example as per step #3 above and if so did it work?

I've noticed that the tool used to bootstrap this project (https://github.com/transitive-bullshit/create-react-library) also has a Typescript scaffolding option. Is this something worth considering as a last resort? (ie re-scaffolding the project using create-react-library with TS enabled)

vaibhavhrt · 2019-06-18T17:26:51Z

@trblackw you can take a look at this repo, I am using ts there, it works (almost) perfectly, vaibhavhrt/react-github-buttons. My project was also bootstrapped using create-react-library but I used the typescript template, and manually updated all dev-dependencies.

trblackw · 2019-06-18T17:29:45Z

@vaibhavhrt Why not just use create-react-app with the --typescript flag? I've worked with TypeScript a lot, mostly with the foundation set up with CRA and have never had issues. @nickmcmillan I'm curious why you used create-react-library?

vaibhavhrt · 2019-06-18T17:39:36Z

@trblackw because it's a npm library not a react website/app. This repo is supposed to be pluggable into some other react app(may or may not be bootstrapped with CRA).
When creating an app we have an index.html but not in a library like this, because it will be used in some app which will have it index.html.
Web app (usually) have minified js, css etc, here in library we don't minify it. So that users can easily debug, and minify, gzip according to their choice.
In library we publish (usually) index.js and index.es.js to enable tree shaking.

trblackw · 2019-06-18T17:45:51Z

Oh, well that makes sense then. Very cool. That’s a bit out of my scope, though, as far as how to go about resolving the current compile issue.

…

On Tue, Jun 18, 2019 at 1:39 PM Vaibhav Vishal ***@***.***> wrote: @trblackw <https://github.com/trblackw> because it's a npm library not a react website/app. This repo is supposed to be pluggable into some other react app(may or may not be bootstrapped with CRA). When creating an app we have an index.html but not in a library like this, because it will be used in some app which will have it index.html. Web app (usually) have minified js, css etc, here in library we don't minify it. So that users can easily debug, and minify, gzip according to their choice. In library we publish (usually) index.js and index.es.js to enable tree shaking. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT25HOLBSNTY4HXDXR7LP3EMVTA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX7M2ZA#issuecomment-503237988>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT27GTU4VTXOAQMPB2I3P3EMVTANCNFSM4HSBJV2Q> .

vaibhavhrt · 2019-06-18T17:52:10Z

I am guessing something is wrong in rollup.config Take a look at the rollup config of my repo and try to see what's different in them. I am not expert enough in ts to figure out the solution just by looking at error, so you try changing things in rollup.config If you can't figure it out I will clone your fork again and take a look.

trblackw · 2019-06-18T17:53:07Z

Okay I’ll take a look. Will get back to you some time tomorrow @vaibhavhrt

…

On Tue, Jun 18, 2019 at 1:52 PM Vaibhav Vishal ***@***.***> wrote: I am guessing something is wrong in rollup.config Take a look at the rollup config of my repo and try to see what's different in them. I am not expert enough in ts to figure out the solution just by looking at error, so you try changing things in rollup.config If you can't figure it out I will clone your fork again and take a look. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9?email_source=notifications&email_token=AIVCT2YP35Q7ULK64LK5AHTP3EOEVA5CNFSM4HSBJV22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX7N72A#issuecomment-503242728>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVCT26EOJ4ZYEJEBJBZWILP3EOEVANCNFSM4HSBJV2Q> .

trblackw · 2019-06-19T21:35:14Z

@vaibhavhrt Haven't had any luck. Maybe you could take a look

vaibhavhrt · 2019-06-20T20:11:36Z

ok, I will look when i get some time

nickmcmillan · 2019-06-24T13:22:14Z

I think I have a working solution. @vaibhavhrt and @trblackw would you mind taking a look at this PR; #12 (which is branched of the work you've both done so far in this current PR)

Basically I re-initialised the create-react-library with TS enabled which provided a working foundation. I then copied the src files over from this PR, fixed a few errors, and it seems to be up and running ok (using the /example project).

@vaibhavhrt I don't think we need lintJs anymore and the eslint plugins that go with it. We can just use create-react-library's testing set up. So I've just made lintJS an alias for test so that Travis tests can pass.

Tucker added 3 commits June 1, 2019 17:08

Add TS dependencies and setup TS (linting) environment

e83970f

convert components/files to TS

4bb760f

define explicit types for variables that were left un-typed

258704c

vaibhavhrt mentioned this pull request Jun 6, 2019

Set up Travis CI #8

Merged

add css extension to path options to pass to includePaths bundle func…

ad032a6

…tion

merge upstream master

681f034

vaibhavhrt suggested changes Jun 12, 2019

View reviewed changes

Tucker added 4 commits June 12, 2019 11:36

add comma dangle and indent rule; remove tslint plugin due to conflic…

30be93b

…t with eslint

add single quotes around lintJs extension target glob

1981342

ensure index.js is deleted

217f4f4

fix file indentation per updated eslintrc

686f791

vaibhavhrt suggested changes Jun 12, 2019

View reviewed changes

src/.eslintrc

@@ -1,5 +0,0 @@

{

"env": {

"jest": true

Copy link

Contributor

vaibhavhrt Jun 12, 2019

Choose a reason for hiding this comment

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

put this line in actual .eslintrc.

vaibhavhrt suggested changes Jun 12, 2019

View reviewed changes

add react-scripts-ts as devDependency and then update the test scripts

612b8af

add tsconfig.test.json

53799ff

vaibhavhrt suggested changes Jun 17, 2019

View reviewed changes

trblackw and others added 4 commits June 17, 2019 16:51

Update src/index.tsx

3e110ee

Co-Authored-By: Vaibhav Vishal <[email protected]>

fix tsconfig to allow tests to pass

0a91bf7

avoid useless spreading over props.style

9ae2110

Merge branch 'master' of github.com:trblackw/react-physics-dragger

4ac01f7

vaibhavhrt approved these changes Jun 18, 2019

View reviewed changes

nickmcmillan mentioned this pull request Jun 24, 2019

Ts setup #12

Merged

nickmcmillan merged commit 4ac01f7 into nickmcmillan:master Jun 27, 2019

Convert core components to TypeScript (Round 2) #9

Convert core components to TypeScript (Round 2) #9

Conversation

trblackw commented Jun 1, 2019

nickmcmillan commented Jun 3, 2019

trblackw commented Jun 3, 2019 via email

trblackw commented Jun 3, 2019 via email

vaibhavhrt commented Jun 12, 2019

trblackw commented Jun 12, 2019 via email • edited Loading

vaibhavhrt commented Jun 12, 2019

trblackw commented Jun 12, 2019 • edited Loading

vaibhavhrt commented Jun 12, 2019

trblackw commented Jun 12, 2019 • edited Loading

vaibhavhrt commented Jun 12, 2019 • edited Loading

trblackw commented Jun 12, 2019 via email

vaibhavhrt commented Jun 12, 2019

trblackw commented Jun 12, 2019 via email

vaibhavhrt left a comment • edited Loading

Choose a reason for hiding this comment

vaibhavhrt Jun 12, 2019 • edited Loading

Choose a reason for hiding this comment

vaibhavhrt Jun 12, 2019

Choose a reason for hiding this comment

trblackw commented Jun 12, 2019 via email

vaibhavhrt commented Jun 12, 2019 via email

trblackw commented Jun 12, 2019

vaibhavhrt left a comment

Choose a reason for hiding this comment

vaibhavhrt Jun 12, 2019

Choose a reason for hiding this comment

vaibhavhrt left a comment

Choose a reason for hiding this comment

vaibhavhrt Jun 12, 2019 • edited Loading

Choose a reason for hiding this comment

trblackw commented Jun 12, 2019 via email

vaibhavhrt commented Jun 12, 2019

trblackw commented Jun 12, 2019

vaibhavhrt commented Jun 13, 2019

trblackw commented Jun 13, 2019

vaibhavhrt commented Jun 13, 2019

trblackw commented Jun 13, 2019 via email

trblackw commented Jun 16, 2019

vaibhavhrt commented Jun 16, 2019

vaibhavhrt left a comment

Choose a reason for hiding this comment

vaibhavhrt Jun 17, 2019

Choose a reason for hiding this comment

trblackw Jun 17, 2019

Choose a reason for hiding this comment

vaibhavhrt Jun 18, 2019

Choose a reason for hiding this comment

trblackw commented Jun 17, 2019

vaibhavhrt commented Jun 18, 2019

nickmcmillan commented Jun 18, 2019 • edited Loading

vaibhavhrt commented Jun 18, 2019

trblackw commented Jun 18, 2019

vaibhavhrt commented Jun 18, 2019

trblackw commented Jun 18, 2019 via email

vaibhavhrt commented Jun 18, 2019

trblackw commented Jun 18, 2019 via email • edited Loading

trblackw commented Jun 19, 2019

vaibhavhrt commented Jun 20, 2019

nickmcmillan commented Jun 24, 2019

trblackw commented Jun 12, 2019 via email •

edited

Loading

trblackw commented Jun 12, 2019 •

edited

Loading

trblackw commented Jun 12, 2019 •

edited

Loading

vaibhavhrt commented Jun 12, 2019 •

edited

Loading

vaibhavhrt left a comment •

edited

Loading

vaibhavhrt Jun 12, 2019 •

edited

Loading

vaibhavhrt Jun 12, 2019 •

edited

Loading

nickmcmillan commented Jun 18, 2019 •

edited

Loading

trblackw commented Jun 18, 2019 via email •

edited

Loading