emmanueltouzery / prelude-ts

Functional programming, immutable collections and FP constructs for typescript and javascript
ISC License
377 stars 21 forks source link

Cleanup package #37

Closed aleksei-berezkin closed 4 years ago

aleksei-berezkin commented 4 years ago

I could not test docgen: first sed -i didn't work normally on my OS; second, when I tried changing params to work, patched TS files did not compile, perhaps again because sed. So could you please test whether it works on your system.

UPD sed -i works differently on my system (it's non-standard param), but anyway testable with local changes.

aleksei-berezkin commented 4 years ago

Well seems like indeed is wrong with docgen here. Will try to figure it out

aleksei-berezkin commented 4 years ago

It's much smaller now. Not sure if tsconfig.json is needed in package, probably not, so didn't include.

$ npm pack
...
npm notice === Tarball Details === 
npm notice name:          prelude-ts                              
npm notice version:       0.8.3                                   
npm notice filename:      prelude-ts-0.8.3.tgz                    
npm notice package size:  255.0 kB                                
npm notice unpacked size: 1.6 MB                                  
npm notice shasum:        a15a652745255f38681443d0faa9429ce576db64
npm notice integrity:     sha512-T4CsfAbG5ym9o[...]aVLnNPNJQ8RPw==
npm notice total files:   98 
...
aleksei-berezkin commented 4 years ago

@emmanueltouzery Hi what do you think of this PR? It's ready for review

emmanueltouzery commented 4 years ago

Yes I saw it, I need to take the time to refresh my knowledge of that part of the code. Sorry, I'll try to review it asap! And thank you for the work, it's much appreciated!

aleksei-berezkin commented 4 years ago

Ok just wanted to make sure you didn't overlook it, thank you so much

emmanueltouzery commented 4 years ago

what's strange is that according to npm pack (i didn't know that command, thank you!) the package in master is 2.5Mb unpacked (and 1.6Mb after your changes). So I'm confused where does npm.com finds the 6.11Mb it's reporting... https://www.npmjs.com/package/prelude-ts

emmanueltouzery commented 4 years ago

yes, this looks good, let's merge this! Thank you!

I now have one last change in mind, a replace-like call (may or may not happen), I'll try to make a PR today if I do it, and then I hope to release a 1.0 version, hopefully this week-end.