TypeStrong / dts-bundle

Export TypeScript .d.ts files as an external module definition
MIT License
308 stars 57 forks source link

Exclude 'external' typings (such as node.d.ts) by default, fixed hang bug, and more #3

Closed poelstra closed 10 years ago

poelstra commented 10 years ago

Hi Bart,

It's become a more extensive change than I had imagined at first, but here it is.

Note that I have changed the default behavior to (hopefully) be more in line with 'generic' use, i.e. exclude external typings by default, don't delete source typings, etc. These are the options you'd need to regain the 'old' behavior:

dts.bundle({
    name: 'cool-project',
    main: 'build/index.d.ts',
    out: 'build/index.d.ts',
    includeExternal: true,
    deleteSourceTypings: true,
});

You may also find verbose: true to be useful :)

If you accept the pull, may I suggest you update the package version, and publish it? Btw: because npm treats 0.x.x versions differently, I'd also suggest bumping to 1.x.x.

Grtz! Martin

Bartvds commented 10 years ago

Looks promising, I'll merge it and check it out. The options are nice in any case.

I see the build got hit by EOL problems but I'll add some .gitattributes to clear that up..

I'm not so sure about v1.0.0, as the special treatment is what we want (I'm looking at this), I think v0.1.0 an then v0.2 etc will do, even with caret ^.

Bartvds commented 10 years ago

Changes are pretty good so far.

I found one bug when I ran it in my projects that use this module; I fixed it easily thanks to the really verbose output :wink:

(something with mainFile resolve)

https://github.com/grunt-ts/dts-bundle/commit/5a95c8e4c9c1f8541a2de26d110f29433f01fb4b#diff-6d186b954a58d5bb740f73d84fe39073L141

Maybe you can grab the lib/index.js file and dump it in your project and see if it all still works.

I've done some cosmetic stuff too, but now I'll add some more tests.

poelstra commented 10 years ago

Thanks for the quick merge! Your additional changes look good to me, I've just ran it on my typings and it works like a charm.

I did indeed also notice the mainFile resolve issue shortly after I pushed, Apparently, I had been testing with absolute paths for mainFile before.

Bartvds commented 10 years ago

We are live at v0.2.0!

I tested the semver and it works fine, even with caret ^, due to the special case mentioned above. This is desirable because at such early state we don't want to bump the major version on every backwards-incompatible change.

Eg: a project had ^0.1.1 dependency and did not get 0.2.0 as expected. This matches with how big projects run too, eg: node itself is still < 1.0.0.

poelstra commented 10 years ago

Nice work!

As long as it is indeed expected to have API-breaking changes in the 'second' version number when the first number is 0, than that is fine by me. I ran across the exact opposite: we bumped a package's second number because we had a non-breaking change (otherwise we'd have bumped the first one to 1), and it didn't get picked up. Only when we looked really carefully, we noted the exception to the rule. Either way, calling something 1.0 is always a subjective matter :)

Bartvds commented 10 years ago

Cool, thanks for using it and send PR's.