{"action":"created","issue":{"url":"https://api.github.com/repos/webpack/webpack/issues/6862","repository_url":"https://api.github.com/repos/webpack/webpack","labels_url":"https://api.github.com/repos/webpack/webpack/issues/6862/labels{/name}","comments_url":"https://api.github.com/repos/webpack/webpack/issues/6862/comments","events_url":"https://api.github.com/repos/webpack/webpack/issues/6862/events","html_url":"https://github.com/webpack/webpack/pull/6862","id":308364449,"number":6862,"title":"Add TypeScript type checking ","user":{"login":"mohsen1","id":543633,"avatar_url":"https://avatars0.githubusercontent.com/u/543633?v=4","gravatar_id":"","url":"https://api.github.com/users/mohsen1","html_url":"https://github.com/mohsen1","followers_url":"https://api.github.com/users/mohsen1/followers","following_url":"https://api.github.com/users/mohsen1/following{/other_user}","gists_url":"https://api.github.com/users/mohsen1/gists{/gist_id}","starred_url":"https://api.github.com/users/mohsen1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mohsen1/subscriptions","organizations_url":"https://api.github.com/users/mohsen1/orgs","repos_url":"https://api.github.com/users/mohsen1/repos","events_url":"https://api.github.com/users/mohsen1/events{/privacy}","received_events_url":"https://api.github.com/users/mohsen1/received_events","type":"User","site_admin":false},"labels":[{"id":572712741,"url":"https://api.github.com/repos/webpack/webpack/labels/PR:%20CI-ok","name":"PR: CI-ok","color":"0e8a16","default":false},{"id":575750680,"url":"https://api.github.com/repos/webpack/webpack/labels/PR:%20reviewed-approved","name":"PR: reviewed-approved","color":"0e8a16","default":false},{"id":572838417,"url":"https://api.github.com/repos/webpack/webpack/labels/PR:%20tests-needed","name":"PR: tests-needed","color":"d93f0b","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":16,"created_at":"2018-03-25T16:38:00Z","updated_at":"2018-04-12T15:13:14Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/webpack/webpack/pulls/6862","html_url":"https://github.com/webpack/webpack/pull/6862","diff_url":"https://github.com/webpack/webpack/pull/6862.diff","patch_url":"https://github.com/webpack/webpack/pull/6862.patch"},"body":"## Summary\r\nWanted to try using TypeScript with `allowJs` and `checkJs` to see if it can improve code correctness. \r\n\r\nWebpack is a good case study for using TypeScript compiler on JavaScript projects. This is still a work in progress and I'm not sure if we can use TypeScript to check for errors without any code modifications. \r\n\r\n## Issues in TypeScript and DefiantlyTyped\r\n\r\n- [x] TypeScript does not allow using `_this` as a variable name. \r\n  * https://github.com/Microsoft/TypeScript/pull/155. \r\n  * Error: ` Expression resolves to variable declaration '_this' that compiler uses to capture 'this' reference.`\r\n  * https://github.com/Microsoft/TypeScript/issues/22866\r\n- [x] Assigning properties to class members is not allowed. even in non strict mode \r\n  * TypeScript does not like this code:\r\n  * `class Foo {}; let f = new Foo(); f.bar = 1` \r\n  * https://github.com/Microsoft/TypeScript/issues/22896 \r\n  * We will initialize members with `undefined` as value in class constructor to surpass those errors \r\n  * [x] https://github.com/webpack/webpack/pull/6883\r\n  * [x] https://github.com/webpack/webpack/pull/6886\r\n  * [x] https://github.com/webpack/webpack/pull/6885\r\n  * [x] https://github.com/webpack/webpack/pull/6884\r\n  * [x] https://github.com/webpack/webpack/pull/6897\r\n  * [x] https://github.com/webpack/webpack/pull/6910\r\n  * [x] https://github.com/webpack/webpack/pull/6974\r\n  * [x] ~~https://github.com/webpack/webpack/pull/6987~~\r\n- [x] Some `@types/tappable` types are not accurate\r\n  * https://github.com/DefinitelyTyped/DefinitelyTyped/pull/24527\r\n- [x] TypeScript confusing class prototype assignments with instance members \r\n  * Errors like `Class 'Module' defines instance member property 'identifier', but extended class 'ExternalModule' defines it as instance member function.`\r\n  * https://github.com/Microsoft/TypeScript/issues/22895\r\n- [x] Webpack is using some undocumented and deprecated Node.js APIs\r\n  * ~~https://github.com/DefinitelyTyped/DefinitelyTyped/pull/24526~~\r\n  * ~~https://github.com/DefinitelyTyped/DefinitelyTyped/pull/24521~~\r\n  * Those definitions can be declared in `declarations.d.ts`\r\n  * In one case we had to use `// @ts-ignore` directive\r\n- [x] TypeScript is too strict about `module.exports` assignments \r\n  * https://github.com/Microsoft/TypeScript/issues/15647\r\n- [x] Type definitions for `vm.runInThisContext` is wrong. it does not accept string filename for options\r\n  * https://github.com/DefinitelyTyped/DefinitelyTyped/pull/24798\r\n- [x] TypeScript can not understand some of existing JSDoc types in Webpack\r\n  * https://github.com/webpack/webpack/pull/6988\r\n  * https://github.com/Microsoft/TypeScript/issues/11597#issuecomment-379566780\r\n- [x] TypeScript does not like the fact that a subclass static member is not conforming to the superclass \r\n  *  https://github.com/webpack/webpack/pull/6989\r\n- [x] TypeScript does not like overriding values with different types and there is no way to have another type than `string` for iterator of `for..in` in `Object.keys`\r\n  * https://github.com/webpack/webpack/pull/7020\r\n## Issues found in Webpack by TypeScript\r\n\r\n- [x] 1. Accessing `compilationDependencies` in `Compilation` (Fix: https://github.com/webpack/webpack/pull/6885) \r\n- [x] 2. Passing 3 args to `WebAssemblyImportDependency` https://github.com/webpack/webpack/pull/6882\r\n- [x] 3. `JavascriptGenerator` has no constructor but getting arguments  https://github.com/webpack/webpack/pull/6881\r\n- [x] 4. [setProperty with too many args](https://github.com/mohsen1/webpack/blob/91f40d1fd802618ec573c881e7050f58fd4e8c45/lib/OptionsDefaulter.js#L42-L59) #6875 #6900 #6899 #6898\r\n- [x] 5. `hideStack` is being appended to `Error` class.\r\n  * Fix https://github.com/webpack/webpack/pull/6932\r\n- [x] 6. Unused `dependencyTemplatesHashMap` weak map https://github.com/webpack/webpack/pull/6922\r\n- [x] 7. Using Array methods on Set https://github.com/webpack/webpack/pull/6950\r\n- [x] 8. Webpack has some invalid calls to Date and JSON APIs https://github.com/webpack/webpack/pull/6982\r\n- [x] 9. `cacheMerged` will not work with more than 2 arguments \r\n  * https://github.com/Microsoft/TypeScript/issues/22975 \r\n  * https://github.com/webpack/webpack/pull/7013 (Thanks to @ooflorent!)\r\n\r\n### When this is complete? \r\nThis is going to be a lot of work to get it to the finish line and I'm not sure if it is inline with the project goals. This is why I'm opening the pull request early. Hoping we can overcome all of those TypeScript issues and make use of TypeScript compiler in Webpack! 🙏 \r\n\r\n#### Only comment changes \r\nThis pull request on its own should not hav **any code changes**. TypeScript type checking should be non-invasive and should not make users to change their code. At most, changes should be **comments only**. If any code change is necessary due to bugs or TS limitations, we are going to propose and merge them in separate pull request.  \r\n\r\n#### TypeScript 2.9\r\nThe amazing TypeScript have fixed some issues with the compiler when checking JavaScript code. Those changes are currently (April 11) in the nightly builds of version 2.9. We _can_ merge while depending on nightly version but it would be nicer to depend to `beta` release in master. We should eventually depend on a stable release.\r\n\r\n@TheLarkInn @DanielRosenwasser  @mhegazy "},"comment":{"url":"https://api.github.com/repos/webpack/webpack/issues/comments/380840856","html_url":"https://github.com/webpack/webpack/pull/6862#issuecomment-380840856","issue_url":"https://api.github.com/repos/webpack/webpack/issues/6862","id":380840856,"user":{"login":"webpack-bot","id":26676291,"avatar_url":"https://avatars0.githubusercontent.com/u/26676291?v=4","gravatar_id":"","url":"https://api.github.com/users/webpack-bot","html_url":"https://github.com/webpack-bot","followers_url":"https://api.github.com/users/webpack-bot/followers","following_url":"https://api.github.com/users/webpack-bot/following{/other_user}","gists_url":"https://api.github.com/users/webpack-bot/gists{/gist_id}","starred_url":"https://api.github.com/users/webpack-bot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/webpack-bot/subscriptions","organizations_url":"https://api.github.com/users/webpack-bot/orgs","repos_url":"https://api.github.com/users/webpack-bot/repos","events_url":"https://api.github.com/users/webpack-bot/events{/privacy}","received_events_url":"https://api.github.com/users/webpack-bot/received_events","type":"User","site_admin":false},"created_at":"2018-04-12T15:13:13Z","updated_at":"2018-04-12T15:13:13Z","author_association":"NONE","body":"<!-- identifier: tests-result -->\n\nIt looks like this Pull Request doesn't include [enough test cases](https://codecov.io/gh/webpack/webpack/compare/0e88e8ab008815707f58c73e4517df4dd33362ef...a3df74b2a173c9cf89671d119710cb3648410920) (based on Code Coverage analysis of the PR diff).\n\nA PR need to be covered by tests if you add a new feature (we want to make sure that your feature is working) or if you fix a bug (we want to make sure that we don't run into a regression in future).\n\n@mohsen1 Please check if this is appliable to your PR and if you can add more test cases.\n\nRead the [test readme](https://github.com/webpack/webpack/blob/master/test/README.md) for details how to write test cases."}}
{"actor":{"display_login":"webpack-bot"}}