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

Switch to Typescript incrementally (test runs ts-node) #9535

Closed
wants to merge 84 commits into from

Conversation

rosskevin
Copy link
Member

@rosskevin rosskevin commented Dec 18, 2017

Supersedes #9517. See #9517 for details.

What's left

  • a few broken tests (1-5 depending on the run)
  • next.js build issue (details below)

Completed

  • js/ts/tsx all work in mocha
  • converted internal utils/styles etc to ts
  • converted a few components for testing
  • converted a few specs for testing
  • fixed known build issues - yarn build runs - updated to babel 7. Effectively, we use tsc like we did flow, and tsc is uninvolved in the build process (but a required precursor).

@rosskevin
Copy link
Member Author

rosskevin commented Dec 19, 2017

And there we have a problem with ts/babel - imports again. import = require is not supported by babel (and shouldn't be). This is a workaround for non-es non-default exports as I understand it.

SyntaxError: src/transitions/Slide.tsx: `import =` is not supported.

@pelotom thoughts on getting around this with babel for compilation? Perhaps there are other options than import = require than I am aware?

https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require

I filed babel/babel#7062 but I think we'll likely need to use tsc to process to es2015 first.

@rosskevin
Copy link
Member Author

The prettier issue is fixed - I didn't change the parser.

@rosskevin
Copy link
Member Author

rosskevin commented Dec 19, 2017

Note - I have a potential fix for import = @require - trying it out now.

@rosskevin
Copy link
Member Author

build:es is working - first pass through the code with babel complete.

@rosskevin
Copy link
Member Author

yarn build works 😄

We are just a bit closer.

rippleVisible: {
opacity: 0.2,
},
export const styles: StyleRulesCallback<TouchRippleClassKey> = (theme: Theme) => ({
Copy link
Member

Choose a reason for hiding this comment

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

FYI, you don’t need the : Theme annotation since you declared the overall type of styles.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done thanks.

@rosskevin
Copy link
Member Author

rosskevin commented Dec 19, 2017

It looks like next.js added jsx support 14 days ago in vercel/next.js#3376

I assume next.js would need to be using babel 7 first (this babel 7 PR has gone uncommented vercel/next.js#3428). And if using babel 7, then there doesn't seem to be any reason why ts|tsx wouldn't work with the way we have approached this and changes just like they did for jsx. I added a comment on the next.js issue vercel/next.js#2391 (comment)

@oliviertassinari oliviertassinari removed their assignment Dec 19, 2017
@rosskevin rosskevin changed the title Switch to Typescript incrementally Switch to Typescript incrementally (test runs ts-node) Dec 20, 2017
@rosskevin rosskevin added the on hold There is a blocker, we need to wait label Dec 20, 2017
@rosskevin
Copy link
Member Author

Closing in favor of #9561. If we want to go back to using ts-node, I'll have to re-apply those changes there.

@rosskevin rosskevin closed this Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold There is a blocker, we need to wait typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants