{"action":"created","issue":{"url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/issues/3615","labels_url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/issues/3615/labels{/name}","comments_url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/issues/3615/comments","events_url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/issues/3615/events","html_url":"https://github.com/borisyankov/DefinitelyTyped/pull/3615","id":56893049,"number":3615,"title":"Split modules react and react/addons; exporting Component<P, S, C> class","user":{"login":"vsiao","id":980673,"avatar_url":"https://avatars.githubusercontent.com/u/980673?v=3","gravatar_id":"","url":"https://api.github.com/users/vsiao","html_url":"https://github.com/vsiao","followers_url":"https://api.github.com/users/vsiao/followers","following_url":"https://api.github.com/users/vsiao/following{/other_user}","gists_url":"https://api.github.com/users/vsiao/gists{/gist_id}","starred_url":"https://api.github.com/users/vsiao/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vsiao/subscriptions","organizations_url":"https://api.github.com/users/vsiao/orgs","repos_url":"https://api.github.com/users/vsiao/repos","events_url":"https://api.github.com/users/vsiao/events{/privacy}","received_events_url":"https://api.github.com/users/vsiao/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-02-07T02:15:03Z","updated_at":"2015-02-09T19:20:47Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/pulls/3615","html_url":"https://github.com/borisyankov/DefinitelyTyped/pull/3615","diff_url":"https://github.com/borisyankov/DefinitelyTyped/pull/3615.diff","patch_url":"https://github.com/borisyankov/DefinitelyTyped/pull/3615.patch"},"body":"So... this PR splits `react-0.13.0.d.ts` and `react-addons-0.13.0.d.ts`, with no shared code or interfaces. That feels bad, but there's a good reason why:\r\n\r\n# The Problem\r\n`React.Exports.Component` doesn't have type parameters (it has the type interface `React.ComponentClass<any, any, any>`, which is not extendable by a `class`). The reason this seems to work right now is because `React.Component` references the class in the global `React` module, and not in `\"react\"` or `\"react/addons\"`. So if you `import react`, `react.Component` is not a class.\r\n\r\n# The Solution\r\nExport `class Component<P, S, C>` directly from `module \"react\"` as well as from `module \"react/addons\"`. This has some unfortunate side effects:\r\n\r\n### Top Level Exports\r\nPreviously we managed sharing between `\"react\"` and `\"react/addons\"` by extending `React.AddonsExports` from `React.Exports`. Now that we need to export a class, we can't use export assignment:\r\n```ts\r\nvar exports: React.Exports;\r\nexport = exports;\r\n```\r\nSo every Top Level API method must be exported individually.\r\n```ts\r\nexport function createClass<P, S, C>(...)\r\nexport function createElement<P>(...)\r\nexport class Component<P, S, C> { ... }\r\n```\r\n\r\n### No Shared Interfaces\r\nWe got away with packing all React-related interfaces into the global `React` module because neither `\"react\"` nor `\"react/addons\"` exported any individual elements. Now that they do, If you use\r\n```ts\r\nimport React = require(\"react\");\r\n```\r\nit clobbers the `React` module definition and its interfaces are no longer accessible. This means that we need to either (a) define global interfaces (`ReactElement`, `ReactComponent`, etc.), which is against DT best practices, or (b) declare interfaces in both `\"react\"` and `\"react/addons\"` modules.\r\n\r\n# Attempted Workarounds\r\nmodule merging:\r\n\r\n```ts\r\ndeclare module \"react/addons\" {\r\n    import React = require(\"react\");\r\n    module React { // <---- (*)\r\n        var addons: { ... };\r\n    }\r\n    export = React;\r\n}\r\n```\r\n\r\n_Doesn't work_. The module to be merged `(*)` is ignored completely.\r\n\r\n# So...\r\n\r\nThis is the conclusion that I came to. We should split up `\"react\"` and `\"react/addons\"`, making sure that changes stay in sync between the two files.\r\n\r\n_Bonus features_:\r\n* fixes `static` properties on modern components (needs type annotations)\r\n* write better tests for Top-Level API, making sure that they preserve type information (eg. `ClassicComponentClass -> ClassicFactory -> ReactClassicElement -> ClassicComponent`)\r\n\r\ncc @pspeter3, @jbrantly"},"comment":{"url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/issues/comments/73571571","html_url":"https://github.com/borisyankov/DefinitelyTyped/pull/3615#issuecomment-73571571","issue_url":"https://api.github.com/repos/borisyankov/DefinitelyTyped/issues/3615","id":73571571,"user":{"login":"jbrantly","id":196944,"avatar_url":"https://avatars.githubusercontent.com/u/196944?v=3","gravatar_id":"","url":"https://api.github.com/users/jbrantly","html_url":"https://github.com/jbrantly","followers_url":"https://api.github.com/users/jbrantly/followers","following_url":"https://api.github.com/users/jbrantly/following{/other_user}","gists_url":"https://api.github.com/users/jbrantly/gists{/gist_id}","starred_url":"https://api.github.com/users/jbrantly/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jbrantly/subscriptions","organizations_url":"https://api.github.com/users/jbrantly/orgs","repos_url":"https://api.github.com/users/jbrantly/repos","events_url":"https://api.github.com/users/jbrantly/events{/privacy}","received_events_url":"https://api.github.com/users/jbrantly/received_events","type":"User","site_admin":false},"created_at":"2015-02-09T19:20:47Z","updated_at":"2015-02-09T19:20:47Z","body":"> the React module and the imports can collide\r\n\r\nI'm not sure I entirely understand the problem with this. Are there any negative side-effects? AFAIK in my example if you did `import React = require('react')` and then used `React` it's true there is potentially some ambiguity between what `React` refers to (the imported module or the global module) but in practice it doesn't really matter because they both mean the same thing. If anything I think it would be more confusing if they *didn't* collide because you could do `import react = require('react')` and usage of both `react` and `React` will compile but only `react` will actually work. But this is just a side-effect of  the recommended approach to supporting both internal and external modules with the same declaration file[1].\r\n\r\nAlso, to be clear, I don't think this point is relevant to the normal/addons separation but more to the internal/external modules separation. I apologize for muddying the waters a bit in this regard. However, I do think we need to keep internal/external module separation in mind.\r\n\r\n> if you have both definitions in your project, require(\"react\"); automatically gives you addons because of module merging\r\n\r\nThat's true and unfortunate. IMO it seems like a worthwhile tradeoff to keep from having repeated declarations, but I could certainly understand if others disagreed. It would be nice if we didn't have to make the tradeoff at all somehow but I don't see another alternative.\r\n\r\nAnother thing to keep in mind is that it's possible this doesn't actually break anything at run time. Assuming you're using both, as you said, it looks like `addons` gets appended to the react export so `require('react')` would actually have the addons assuming `require('react/addons')` had also been called. But that isn't guaranteed. Just food for thought. See https://github.com/facebook/react/blob/master/src/browser/ReactWithAddons.js\r\n\r\n> Maybe we can have a global module for react interfaces (ReactApi?) and then have the modules export their own individual things\r\n\r\nI think this could work but might be a little unwieldy. I assume you mean something like:\r\n\r\n```\r\nimport react = require('react');\r\n\r\nvar elm: ReactApi.ReactElement<any> = react.createElement(...)\r\n```\r\n\r\nNot entirely sure how I feel about that. I think it might lead to more confusion than is necessary.\r\n\r\n----\r\n\r\nWhat I really don't want to see is the definitions being repeated into a bunch of files. I could foresee repeating in 3 files: one for external modules, one for external modules with addons, and one for internal modules. While that may model everything really well it seems like overkill to me. I think we need to make some concessions. TypeScript Handbook [1] recommends putting both internal and external module definitions in a single file. This has the concession that if you're using external modules, the internal module definitions are still available but the user just has to be aware not to use them. I think we can apply a similar concession to the normal/addons separation. If you're not using addons, fine, don't include that definition file and you don't get it. If you are, then include the right definition file and you get addons everywhere.\r\n\r\n[1] \"Global / External-agnostic Libraries\" http://www.typescriptlang.org/Handbook#writing-dts-files-examples"}}