support typescript compilation

[gugu]

The code is still written in JS, but is compiled with tsc to dist directory.

After this it is possible to switch gradually every js file

[cjbarth]

Other than convention, what is the advantage of the compilation target being dist instead of remaining lib?

[gugu]

@cjbarth there are three choices:

1. We can keep lib folder as a source root and dist as compiled files root
2. We can keep lib folder as a JS root and src as TS root
3. We can keep both TS and JS files in the same folder

Choices 1 and 2 are similar, I can change dist to lib if you want. Choice 3 will pollute lib folder with extra files, which will mislead people, but it also a choice.

What do you think about it?

[cjbarth]

I'm interested in others opinions, but it seems that keeping lib as the compiled target would make sense for consistency going forward. In that case, I would advocate for a src folder and keep lib as the compilation target. This would require a one-time move of all the files from the lib folder to src. It would also mean that the lib folder remains empty in the repository. I would also advocate for a one-time rename of all the .js files to .ts; no content would have to change. If these two operations are done in different commits, it should preserve the git change history.

The result being that, after compilation, the project looks the same as it does now, except there will be a src folder filled with .ts files that are just the same as the .js files we have now. The consumer doing an npm install passport-saml wouldn't notice any difference because we wouldn't npm publish the src folder (.npmignore file needed).

[gugu]

@cjbarth done

I've used files in package.json instead of .npmignore because of:
https://medium.com/@jdxcode/for-the-love-of-god-dont-use-npmignore-f93c08909d8d

[cjbarth]

This all looks good to me, though I haven't pulled the code and tested it. Since this represents a change to the organization of the project, I'd like to get one more set of eyes on this before it is merged in.

[markstos]

I understand that the purpose of || true here is to convert a linting "failure" exit code to a "success" exit code. Effectively, it turns linting errors into warnings, which is fine for now. I confirmed the test suite passes, but there are many linting errors converted to warnings. I see that most errors might be automatically fixed with eslint --fix.

[markstos]

Looks good to me. I like supporting the incremental strategy and introducing the linting. I also like that the PR didn't modify the test suite at all, confirming the change is compatible with our test suite.

[cjbarth]

@gugu Thank you very much for following through on this. I look forward to https://www.npmjs.com/package/@types/passport-saml getting merged into this project now so we can remove those externally maintained definitions.