Skip to content

Allow variable merge with namespace when value of variable is a allowed merging declaration #45975

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

Closed
4 of 5 tasks
FilipeBeck opened this issue Sep 20, 2021 · 7 comments
Closed
4 of 5 tasks
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@FilipeBeck
Copy link

Suggestion

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

It is not allowed merging namespace with variables. But, in some cases, it should be when the value itself is a allowed merging.

This is allowed:

function foo() {
  // ...
}
namespace foo {
  export const bar = 'bar'
}

class Foo {
  // ...
}
namespace Foo {
  export const bar = 'bar'
}

But this is not:

const foo = function() {
  // ...
}
namespace foo {
  export const bar = 'bar'
}

const Foo = class {
  // ...
}
namespace Foo {
  export const bar = 'bar'
}

As class A {... and const A = class {... are equivalent, it should compile with no erros.

@andrewbranch andrewbranch added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript Working as Intended The behavior described is the intended behavior; this is not a bug and removed Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Working as Intended The behavior described is the intended behavior; this is not a bug labels Sep 23, 2021
@andrewbranch
Copy link
Member

Is there a scenario where you need to use const A = class instead of class A?

@FilipeBeck
Copy link
Author

FilipeBeck commented Sep 23, 2021

With class, no until now, but I put it to give it more scope. It's common for me to have problems with function type vars, like here: https://github.com/FilipeBeck/react-audio-engine/blob/d5e90828d960e8969d2bc4cb41537d53868ad3c6/src/reconciler/Components.ts#L80-L96

I have:

const Scene: React.FC< ...
namespace Scene { ...

And I cannot change to:

function Scene(props: { children ...

Because Scene is the return of React.forwardRef

@andrewbranch
Copy link
Member

But I don’t think that helps you, because we couldn’t know that the return of React.forwardRef is safely mergeable with anything else. The only possible way I can see this being allowed (and I’m still unsure about it) is if the initializer is a function expression or class expression, which means you would be able to rewrite it as a function declaration or class declaration.

@FilipeBeck
Copy link
Author

Yes, I'm realizing it now. It is not possible to guarantee that the return value does not have other properties than those declared in the interface, as long as it respects the interface.

The only possible way I can see this being allowed (and I’m still unsure about it) is if the initializer is a function expression ...

Maybe, it can still be useful for scenarios with arrow functions (although it doesn't solve the problem that made me open the suggestion)

@FilipeBeck
Copy link
Author

FilipeBeck commented Sep 27, 2021

In this code, I was trying to re-export a const enum and, as const enum does not generate code, I solved it this way:

export const Scene: React.FC<Scene.Props> & { LatencyCategory: typeof ATOM.Scene.LatencyCategory } = (
  React.forwardRef((props, ref) => {
    return React.createElement(Tag.SCENE, { ...props, ref }, props.children)
  }) as any
)

This works even if the variable has a property with the same name, as seen here

Would allowing merge with const enum violate the design goals?

@FilipeBeck
Copy link
Author

How const enum meets my needs and I always use const enum when possible, I solved creating a interface with same name: https://github.com/FilipeBeck/react-audio-engine/blob/8d61643a9a1b64ba7379628482ba102acc5a0a42/src/reconciler/Components.ts#L87-L106.

That way I don't run the risk of overwriting any additional property returned.

const Scene: Scene = React.forwardedRef() // ...
interface Scene extends React.FC {
  LatencyCategory: typeof ATOM.Scene.LatencyCategory // Is a `const enum`
}
namespace Scene { // ...

Closing the issue. Maybe I open another only about arrow function in the future to allow this:

const MyComponent: React.FC<MyProps> = props => {
  // ...
}
namespace MyComponent {
  // ...
}

@FilipeBeck
Copy link
Author

If it's useful for someone else, better discuss it in a separate and more specific issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants