feat(webpack): Add webpack-dev-server, add yarn.lock #22
Conversation
There was a problem hiding this comment.
Hi @ChinW, thank you very much for this PR! Absolutely is a good feature for the starter! Great job!
I've requested some small changes. About the 2 spaces thing, that's something that could be improved adding an .editorconfig. I've opened #24 for that. In the meanwhile, just make sure you use 2-space indentation everywhere.
Thanks!
webpack.config.ts
Outdated
| const dev = !(env && env === 'production'); | ||
| const entry = dev ? [ | ||
| // 'react-hot-loader/patch', | ||
| 'webpack-dev-server/client?http://localhost:8081', |
There was a problem hiding this comment.
Please use 2 space indentation according to tslint
webpack.config.ts
Outdated
| ] | ||
| ], | ||
| devServer: { | ||
| hot: true, |
webpack.config.ts
Outdated
| template: join(__dirname, 'src/template/common.html'), | ||
| hash: true, | ||
| chunks: ['common', 'index'] | ||
| }), | ||
| new TypedocWebpackPlugin( |
There was a problem hiding this comment.
For dev we don't need to run Typedoc anymoore. Could you make it that it only runs on production?
src/template/common.html
Outdated
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> |
|
@alexjoverm Thanks for your comments 馃憤 I have modified them, please check ;) |
There was a problem hiding this comment.
@ChinW sorry I have a small additional change that I haven't realized before.
Additionally, I have a concern. Since this boilerplate is for writing TS libraries, it is not related to HTML and that shouldn't be part of the source code or the dist (now when running npm run build an index.html is generated there). So, my suggestion:
- In webpack.config make the HTMLWebpackPlugin only execute on dev
- Move template/common.html out of src folder. What about in the root (/template/common.html)?
What do you think about this? Any other suggestions?
Thanks a lot for your time, and sorry for the extra work, but I'd like to keep the quality standard of the project :)
webpack.config.ts
Outdated
| /** | ||
| * Update this variable if you change your library name | ||
| */ | ||
| const libraryName = '--libraryname--' | ||
| const plugins = [ | ||
| new CheckerPlugin(), | ||
| new webpack.HotModuleReplacementPlugin(), |
There was a problem hiding this comment.
Also, we don't need the Hot Module replacement when building for production. Can you make it only for development?
|
@alexjoverm You are most welcome! I have learned a lot from this project 馃憤 Please check the update. And for the I will update this if you still think it is not necessary in this case ;) And any problems pls let me known, I am very willing to do some help, updating is a kind of advancing to me 馃嵒 |
There was a problem hiding this comment.
@ChinW Glad to see your high motivation :)
Supposedly the generated file should work as development, but I see your point and probably some people would like to do some manual testing with the bundles.
In that case, I suggest to create a .npmignore file with dist/index.html in it, since we don't want that index.html to be distributed when the people install the library created with this starter. Check this for more info
With that and a what I wrote in the comments (the deleted src/library and the renaming to index.html) we're done for this PR
src/library.ts
Outdated
| @@ -1,3 +0,0 @@ | |||
| export default class DummyClass { | |||
There was a problem hiding this comment.
Apparently you removed this folder and file. Please revert that, is there on purpose, otherwise you'll see running npm t would fail.
template/common.html
Outdated
| @@ -0,0 +1,11 @@ | |||
| <!doctype html> | |||
There was a problem hiding this comment.
Can we rename this to index.html? In order to keep consistency with the dist/index.html generated file.
|
@alexjoverm Thanks for your time :D I have modified these parts, please check |
Hi contributors,
I am really appreciated for your work, it does help me have a quick start to TS! I am wondering whether it would be helpful by adding webpack-dev-server into this starter? Something likes npm run dev, which allows us to start a server, so that we can have features like hot-reload, and debugging ts with WebStorm/VSC, etc.
I hope this could be helpful. and it's my opinion of course. Up to you 馃憤
--- New Package Script ---
npm run dev: Run a server at localhost:8081 for quick development