-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
TypeScript support #2815
TypeScript support #2815
Conversation
try { | ||
return require('react-scripts-plugin-typescript').tsc; | ||
} catch (e) { | ||
return require('typescript'); |
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.
What if this throws, too?
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.
This would only happen post eject, where throwing would happen if the user had malformed node_modules; by default this will "work". We could add a helpful message, I suppose.
@@ -290,3 +292,5 @@ module.exports = { | |||
hints: false, | |||
}, | |||
}; | |||
|
|||
module.exports = applyPlugins(base, ['typescript'], { path, paths }); |
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.
Maybe expand the whitelisted plugins into multiple lines starting from the first plugin, to minimize future diffs?
module.exports = applyPlugins(base, [
'typescript',
], { path, paths });
Diff:
module.exports = applyPlugins(base, [
'typescript',
+ 'sass',
], { path, paths });
Versus:
-module.exports = applyPlugins(base, ['typescript'], { path, paths });
+module.exports = applyPlugins(base, [
+ 'typescript',
+ 'sass',
+], { path, paths });
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.
This is controlled by prettier, but I agree that it's a good idea.
We're going to be adding more meta information (and probably move where this is) -- we will need plugin name and plugin version supported: { name: 'typescript', version: '1.x' }
.
Once this is turned into an object it'll probably break onto single lines.
pushExclusiveLoader, | ||
} = require('react-dev-utils/plugins'); | ||
|
||
function apply(config, { path, paths }) { |
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.
Just curious: is there any reason this isn't just const path = require('path')
, but passed into the plugin instead?
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.
When inlining the plugin during eject the transform is not smart enough to pull required dependencies/packages; so for now required packages must be passed in.
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.
Ah makes sense! Is react-dev-utils/plugins
a little special then, that you're able to require it just above?
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.
Yeah! Those methods get removed entirely when ejecting -- that package should be the only package that is ever required inside the plugin(s).
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.
@Timer - good stuff here. Question- is there a time we think the plugin system #2784 is going to be added? And if so, how do you think something like this will compare to https://github.com/wmonk/create-react-app-typescript ? Keep up the good work, thanks! |
I am using Welcome to have a look and give it a try zc-react-scripts |
@davidwparker I think the |
@Timer any news on this? Or it has to land after plugin system? |
Is there a plan to have this merged? TypeScript support would be awesome. |
(create react app + typescript + antd) without eject and without using Babel. https://github.com/qinyang1980/create-react-antd-app.git |
@qinyang1980 it's repository supports tree shaking of typescript files in build application? Just I heard so react-create-app doesn't support that feature in building |
I made an alternative PR adding TypeScript using Babel 7: #4837 |
@Timer, Do you have a status update on this? Is it still planned? Is there an ETA? |
@devuxer please follow along here: |
We're very interested in bringing TypeScript support. I will either finish this PR or merge #4837 provided everything works properly. |
Closing in favor of #4837. |
TypeScript is now officially supported as of Create React App 2.1. Read the release notes to get started! |
This includes PR #2784.
Here's an example of a working plugin, which enables TypeScript support for development, production, and testing.
Click here to see the diff without PR #2784.