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

Support TypeScript namespace #8941

Open
xiaoxiangmoe opened this issue May 2, 2020 · 3 comments
Open

Support TypeScript namespace #8941

xiaoxiangmoe opened this issue May 2, 2020 · 3 comments

Comments

@xiaoxiangmoe
Copy link
Contributor

xiaoxiangmoe commented May 2, 2020

https://github.com/welldone-software/why-did-you-render

You can also manually track any component you want by setting whyDidYouRender on them like this:

function BigListPureComponent() {
  return <div>
    //some heavy component you want to ensure doesn't happen if its not neceserry
  </div>
}
BigListPureComponent.whyDidYouRender = true

In TypeScript, if we want whyDidYouRender be readonly, we should write:

function BigListPureComponent() {
  return <div>
    //some heavy component you want to ensure doesn't happen if its not neceserry
  </div>
}
namespace BigListPureComponent {
  export const whyDidYouRender = true;
}

But CRA doesn't support namespace now.

@paolostyle
Copy link

As for namespaces, Babel has somewhat limited support: https://babeljs.io/docs/en/babel-plugin-transform-typescript#impartial-namespace-support and it is disabled by default. However I've never found a reason to use them, especially in React codebases.

As for your issue, these aren't equivalent at all. In fact your second example wouldn't even compile in TypeScript compiler, because in the output it would have to redeclare BigListPureComponent which is a const. It would work if you used function, not const, but still, it's 3 lines of code instead of one and you get additional IIFE in the output for exactly the same effect. There are no upsides to the second solution. Your first example compiles completely fine under strict mode so I personally don't see any reason to meddle with namespaces support.

@xiaoxiangmoe
Copy link
Contributor Author

xiaoxiangmoe commented May 3, 2020

You are right. I change const into function now.

This is one of the standard USES of namespace.
The standard TypeScript practice for adding readonly attributes to a function instance is to use namespace with export const.

Another reason is that our existing code base, which has a hierarchical relationship and contains both type and value, has already used namespace.

@alarbada
Copy link

As for namespaces, Babel has somewhat limited support: https://babeljs.io/docs/en/babel-plugin-transform-typescript#impartial-namespace-support and it is disabled by default

Apparently since v7.13.0 it defaults to true. Is it expected in a future cra release to allow ts namespaces by default then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants