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

automated test to verify typescript compiler builds of resulting TS file #18

Closed
zewa666 opened this issue May 29, 2018 · 7 comments
Closed
Labels

Comments

@zewa666
Copy link
Contributor

zewa666 commented May 29, 2018

with the latest release and combination with a newer TypeScript version, there was a new issue introduced (see PR #17). It would be great if we could add an integration test to make sure this type of issues is getting caught.

Besides that, changing return types of functions is considered a breaking-change so the latest release really should have been bumped with 0.2.0 due to non RC policy with SemVer instead of a major bump

@pjmolina
Copy link
Contributor

Thanks for reporting @zewa666 & @stifflerus

Tried to reproduce with tsc 2.7.1 and 2.8.3 and I was not able to reproduce it yet.
We need to create a repro-test to make the problem evident.

Could provide a sample of:

  1. how you are importing the parser
  2. the tsconfig.json settings you are using

@pjmolina
Copy link
Contributor

pjmolina commented May 29, 2018

Error unnoticed introduced during PR #8 (cc @yokljo)
Fixed at PR #20
Published as npm module version: 0.2.0
Keeping this issue open till adding a repro-test.

@stifflerus
Copy link
Contributor

The bug is present when the typescript compiler is invoked with the --declaration option. This makes typescript generate .d.ts files.

I added this option to the compile:sample npm script, and found an additional export that was missed in PR #20.

However, now the build fails because the declaration files do not pass lint:samples. It believe it would be appropriate to exclude these from tslint.

tslint output/*.ts matches the declaration files also (*.d.ts)
The solution I came up with is tslint `find output -not -iname *.d.ts -iname *.ts` .
There is probably a more elegant way of doing that and I am open to suggestions.

@stifflerus
Copy link
Contributor

Opened PR #21

@zewa666
Copy link
Contributor Author

zewa666 commented May 30, 2018

Good idea @stifflerus, although in the current form the linter isn't automatically started, unless the developer thinks about it. I'd recommend adding Husky with a prepush/precommit hook that will help contributors as well to check for linting issues.

@pjmolina can you flag the 0.14.0 version as deprecated? That should make it easier for people having auto-patch-upgrades enabled, to detect the breaking change easier, with a notice to upgrade to v.0.2.x

@pjmolina
Copy link
Contributor

pjmolina commented May 30, 2018

Fixed with PR #21 tsc --declaration did the trick to expose the issue.
Marked v. 0.1.14 as deprecated.
Published as v. 0.2.1
Thank you @zewa666 & @stifflerus

@trusktr
Copy link

trusktr commented Dec 21, 2019

I've opened a request to fix issues like these in TypeScript by bringing declaration files to parity with language features. microsoft/TypeScript#35822

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

4 participants