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

revisit --noUnusedLocals with "export types can't be named" #13694

Closed
unional opened this issue Jan 26, 2017 · 4 comments
Closed

revisit --noUnusedLocals with "export types can't be named" #13694

unional opened this issue Jan 26, 2017 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@unional
Copy link
Contributor

unional commented Jan 26, 2017

TypeScript Version:
2.1.5 and nightly (2.2.0-dev.20170125)

Code

// actions.ts
export interface FluxStandardAction<Payload, Meta> {
  type: string,
  payload: Payload,
  meta: Meta
}

export function createActionCreator<Payload, Meta>(type: string)
  : (payload?: Payload, meta?: Meta) => FluxStandardAction<Payload, Meta> {
  return (payload: Payload, meta: any) => {
    return createAction(type, payload, meta)
  }
}

export function createAction<Payload, Meta>(type: string, payload?: Payload, meta?: Meta)
  : FluxStandardAction<Payload, Meta> {
  return { type, payload, meta }
}

// consume.ts
import { FluxStandardAction, createActionCreator } from './actions'

interface AuthenticatePayload {
  authenticating: boolean
  authenticated: boolean,
  username?: string,
  errorMessage?: string
}

export const authenticate = createActionCreator<AuthenticatePayload, void>(AuthenticateActionType)

// type of `authenticate()`:
// (payload?: AuthenticatePayload | undefined, meta?: void | undefined)
//   => FluxStandardAction<AuthenticatePayload, void>

I would like to enable --noUnusedLocals but the "export types can't be named" issue keep preventing me from doing so.

Above is a simple scenario that involved a higher order function createActionCreator().
The logic is pretty simple and they function it returns is not hard to understand either.

If I turn on --noUnusedLocals, the consume.ts will fail because FluxStandardAction is not explicitly used.
But if I explicitly define the type of const authenticate, it is very redundant.

On one hand, the author of createActionCreator() may create a type for the return value to fix this:

export type ActionCreator = <Payload, Meta>(payload?: Payload, meta?: Meta)
  => FluxStandardAction<Payload, Meta>

// consume.ts
import { createActionCreator, ActionCreator } from './actions'

...
const authenticate: ActionCreator
  = createActionCreator<AuthenticatePayload, void>(AuthenticateActionType)

However, not all package authors would do that, leaving the consumer with very verbose code with not much value gain, IMO.

As an alternative, maybe we can have an inline comment to disable checks for that particular import?

import {
  // ts:disable-next-line noUnusedLocals
  FluxStandardAction,
  ... } from './actions'

...
@mhegazy
Copy link
Contributor

mhegazy commented Jan 26, 2017

Should not this be covered by #9944?

@unional
Copy link
Contributor Author

unional commented Jan 26, 2017

If we are going to resolve #9944 by auto-import and eliminating the need for this extra import, then yes. It will be covered.

On the other hand, I think the above code discover a bug. As I think of it, const authenticate: ActionCreator = ... should not work because it internally still uses FluxStandardAction and it is still not named. So I guess this code will blow up when imported as a library?

It is currently compiles fine.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 26, 2017

Suppressing --noUnusedLocals errors based on what the declaration emitter needs seems like a very complicated task. I would say #9944 should solve this.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@mhegazy mhegazy added Duplicate An existing issue was already created and removed Needs Investigation This issue needs a team member to investigate its status. labels Feb 6, 2018
@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants