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

[Feedback] - Use TypeScript for config/ and scripts/ and use rollup-plugin-uglify-es to target es6 and run tests in browser too #130

Open
cancerberoSgx opened this issue Nov 17, 2018 · 1 comment
Labels

Comments

@cancerberoSgx
Copy link

Just a feedback:

  • since this is a typescript related library, It would be awesome that the devtools also use TypeScript. In particular config/rollup.config.js is hard to follow due to the jsdoc. For scripts you can use ts-node for run the typescript directly but you will need to transpile them in order to the tools like rollup, jest, etc to "undertand" the config (or maybe not if using valid js?)

  • you are targeting es5 which is fine but for those which can target more than that it won't work because rollup-plugin-uglify doesn't understand es6. using rollup-plugin-uglify-es will work for all.

  • I think jest-puppeteer can be easily added to run the specs in the browser too

  • (more ambitious) would be awesome to use yeoman generator to ask: library name, target js version, if install jest and tests or not, if install lint tool, travis, etc. In my case I only needed the tsc & rollup part. Maybe this could be another project and use this one as a template

Just a feedback, the tool is very helpful and the project has lot of quality. I was looking for building libraries written with typescript, bundling in different module formats and didn't found much since in general people bundle applications not libraries . Congrats, keep it up - you can close this. Sorry for the noise

@Hotell
Copy link
Owner

Hotell commented Dec 12, 2018

Hey @cancerberoSgx ! Thanks a lot for thorough feedback. much appreciated! ❤️

Here are my thoughts on this:

since this is a typescript related library, It would be awesome that the devtools also use TypeScript. In particular config/rollup.config.js is hard to follow due to the jsdoc. For scripts you can use ts-node for run the typescript directly but you will need to transpile them in order to the tools like rollup, jest, etc to "undertand" the config (or maybe not if using valid js?)

All tools are using typescript, but within vanilla JS via checkJS flag. I don't think it's good idea to introduce unnecessary dependency like ts-node which adds unneeded level of abstraction just for simple things like config and init scripts.

Yes I agree that JSDocs are kinda verbose but that's the tax for not using ts-node and I'm fine with that :)

you are targeting es5 which is fine but for those which can target more than that it won't work because rollup-plugin-uglify doesn't understand es6. using rollup-plugin-uglify-es will work for all.

output of this package are various formats:

  • umd bundle: for nodejs and old browsers ( uses umd module format and transpiles to es5 )
  • esm files: uses es2015 modules and code is transpiled to es5, wihtout minification. that's the job of your bundler of preference ( it will be properly tree-shaked as well )
    image
  • esm2015 files: uses latest js ( es2018 ). You can target your bundler to use these files instead of es5
    image
  • esm bundle: inlined bundle to one file, which uses latest js (es2018). also minified version is provided which uses rollup-plugin-terser es6+ aware minifier
    image

I think jest-puppeteer can be easily added to run the specs in the browser too

That's interesting idea. We should make this opt-in I guess, during package initialization. But for for now I'm leaving this "advanced" options for consumer to implement on their own, as someone uses this to create node packages, where browser testing doesn't make any sense (also e2e testing solution)

(more ambitious) would be awesome to use yeoman generator to ask: library name, target js version, if install jest and tests or not, if install lint tool, travis, etc. In my case I only needed the tsc & rollup part. Maybe this could be another project and use this one as a template

PR is already submitted and will be merged today ;)


does this make more sense now ?

Cheers!

Hotell added a commit that referenced this issue Dec 12, 2018
* feat(scripts): use init on postinstall
* feat(scripts): add vscode settings prompt
* ci: use latest yarn
* ci: don't run postinstall on CI
* chore: bump deps
* refactor: use ambient fiels for local types
* feat(templates): add templates and their setup
* feat(config): add replace-in-file types
* fix(scripts): fix init files modifications
* fix(scripts): fix templates setup
* feat(scripts): add additional prompt questions
* fix(scripts): properly handle vscode setup
* fix(scripts): initialize hooks properly
* ci: fix yarn install
* fix(scripts): resolve ts errors
* feat(vscode): add more settings
* fix(scripts): use githubName in readme template
* docs: add setup video
* feat: add index barrel tsd like header
* refactor(scripts): reorder modifyFiles array

BREAKING CHANGE:
Now TS >= 3 is used which may break some packages that desperately need to support lower versions

Closes #142 #145 #130
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants