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 template #963

Merged
merged 10 commits into from
Jan 18, 2019
Merged

Conversation

swyxio
Copy link
Contributor

@swyxio swyxio commented Jan 11, 2019

Description

add typescript template. mostly a direct port of the basic template to typescript, but with aliases set up properly.

Motivation and Context

make it easier to use RS with TS

Types of changes

  • Refactoring/add tests (refactoring or adding test which isn't a fix or add a feature)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • My changes have tests around them

@tannerlinsley
Copy link
Contributor

I think the plugin takes care of some more things you have manually defined. Might want to check: https://gitlab.com/Vinnl/react-static-plugin-typescript/blob/master/src/node.api.ts

@swyxio
Copy link
Contributor Author

swyxio commented Jan 11, 2019

ok i can remove extensions: ['.js', '.jsx', '.ts', '.tsx']

from what i can tell that is the only line i can take out tho... unless i simplify the webpack.config.js insertion process?

@tannerlinsley
Copy link
Contributor

tannerlinsley commented Jan 11, 2019 via email

@swyxio
Copy link
Contributor Author

swyxio commented Jan 12, 2019

alright simplified a buncha stuff

@tannerlinsley
Copy link
Contributor

What else could we do to avoid the API file?

@swyxio
Copy link
Contributor Author

swyxio commented Jan 12, 2019

are they a bad thing? theyre the canonical place to modify webpack config right?

@tannerlinsley
Copy link
Contributor

tannerlinsley commented Jan 12, 2019 via email

@tannerlinsley
Copy link
Contributor

If you're reasonably convinced this is how far we can push things into the plugin, then let me know and we'll get this merged.

@swyxio
Copy link
Contributor Author

swyxio commented Jan 16, 2019

sorry been busy with the netlify allhands past few days. gonna try a few things and see - my lack of complete knowledge of the RS api (and imperfect knowledge of webpack, its always been a weak point of mine) is probably hindering me here from a totally elegant solution but we can definitely improve this over time.

already used it for my podcasting site btw

@swyxio
Copy link
Contributor Author

swyxio commented Jan 16, 2019

ok so i figured I was only using the node.api for path aliases which are frankly optional so i could just take them out. if it were to be a plugin option it would have to be added upstream to react-static-plugin-typescript by @Vinnl.

anyway - removing them made it a lot simpler, and brings us closer to just doing what the basic template does but in typescript!

Copy link
Contributor

@Vinnl Vinnl left a comment

Choose a reason for hiding this comment

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

Thanks for pinging me @sw-yx :)

Aliases are actually issue #1 for the TypeScript plugin - when support lands, it should be automatic (as in: paths you define in your TypeScript config should Just Work). If you think it's important that it's supported then I could work on that this weekend, as it could save you some documentation. If so, let me know.

"@types/react-hot-loader": "^4.1.0",
"@types/webpack-env": "^1.13.6",
"convert-tsconfig-paths-to-webpack-aliases": "^0.9.2",
"ts-loader": "^5.3.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

If you add the plugin, you shouldn't be required to install TypeScript and a loader yourself. (And in any case, the plugin doesn't use ts-loader :) )

"paths": {
"@components/*": ["src/components/*"]
},
"typeRoots": ["./node_modules/@types"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think typeRoots is necessary? For reference, see the tsconfig at https://www.npmjs.com/package/react-static-plugin-typescript#usage

@swyxio
Copy link
Contributor Author

swyxio commented Jan 16, 2019

ohhh nice! ok that can work. and thanks for the tips, will update

@Vinnl
Copy link
Contributor

Vinnl commented Jan 16, 2019

I could work on [alias support] this weekend

It was more involved than I estimated it to be, but on the plus side, I did most of the work tonight. I still have to clean it up for actual release (probably still this weekend), but it'd be great if you could test it using the template before that. You can install it using:

npm install --save-dev react-static-plugin-typescript@1-support-aliases

@swyxio
Copy link
Contributor Author

swyxio commented Jan 16, 2019

wowww. i'm so glad i tagged you lol. trying now.

@swyxio
Copy link
Contributor Author

swyxio commented Jan 18, 2019

@Vinnl it works! waiting for whenever you are ready to update your plugin, then i'll push/merge this template with the final version

@Vinnl
Copy link
Contributor

Vinnl commented Jan 18, 2019

Great! Coincidentally I just finished adding tests and such. It's building now, when that's finished I'll release 3.1.0 with alias support. I'll ping you when that's done :)

(Don't forget to update the README btw! :) )

@Vinnl
Copy link
Contributor

Vinnl commented Jan 18, 2019

@sw-yx It's been released! V3.1.0.

@swyxio
Copy link
Contributor Author

swyxio commented Jan 18, 2019

ok thanks Vinnl you rock. as far as i can tell i'm ready to merge this. pinging @tannerlinsley in case any final comments.

@tannerlinsley
Copy link
Contributor

This is great you guys. Seriously, so good.

@tannerlinsley
Copy link
Contributor

The only thing left is to add it to this page: https://github.com/nozzle/react-static/tree/master/packages/react-static/templates

@swyxio
Copy link
Contributor Author

swyxio commented Jan 18, 2019

ok, updated and added a mention in that README

@tannerlinsley tannerlinsley merged commit d51616a into react-static:master Jan 18, 2019
@tannerlinsley
Copy link
Contributor

Merged! Thanks guys. This is just awesome.

@tannerlinsley
Copy link
Contributor

It should now be available in the latest release 🎉

@Vinnl
Copy link
Contributor

Vinnl commented Jan 19, 2019

Great, thanks! Unfortunately, I did still run into an error when trying it out:

$ react-static start
  Error: /tmp/my-static-site-ts/static.config.js:1
  (function (exports, require, module, __filename, __dirname) { import axios fro  m 'axios';
                                                                       ^^^^^
  SyntaxError: Unexpected identifier

This is in static.config.js, so not a TypeScript file. However, it does not occur with the basic template. Any of you have any idea what that could be?

@swyxio
Copy link
Contributor Author

swyxio commented Jan 20, 2019

solved in the related issue #980

@shamrin
Copy link
Contributor

shamrin commented Jan 29, 2019

I've tried typescript template, but it failed right away: #989.

Update: Upgrading yarn helped solve my problem.

@swyxio swyxio deleted the addTStemplate branch January 29, 2019 07:21
@ngocdaothanh
Copy link
Contributor

TypeScript template does not support image import:
#1589

I guess it's missing Webpack config?

@SleeplessByte
Copy link
Member

Thanks for opening a new issue. It's not good practice to comment on a PR merged over 2 years ago.

@react-static react-static locked as resolved and limited conversation to collaborators Mar 21, 2021
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.

None yet

6 participants