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

Feature request: Track references of string literals on keys and vice-versa for better intellisense #45549

Open
5 tasks done
devanshj opened this issue Aug 23, 2021 · 5 comments
Labels
Bug A bug in TypeScript Domain: Refactorings e.g. extract to constant or function, rename symbol Effort: Difficult Good luck. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this
Milestone

Comments

@devanshj
Copy link

devanshj commented Aug 23, 2021

Suggestion

πŸ” Search Terms

rename string literals, refactor

βœ… Viability Checklist

  • 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

Playground for code below

declare const f1: <T>(a: T, b: { [K in keyof T]: T[K] }) => void

f1({
  foo: () => {}
}, {
  foo: () => {}
})
// ctrl+click on b's foo, it navigates to a's foo -- nice!
// ctrl+click on a's foo, it shows b's foo as a reference -- nice!
// renaming one foo renames the other foo too -- nice!

declare const f2: <T>(a: T, b: keyof T) => void

f2({
  foo: () => {}
}, "foo")
// ctrl+click on b's foo, does nothing -- can be improved by navigating to a's foo!
// ctrl+click on a's foo, does nothing -- can be improved by showing b's foo as a reference!
// renaming one foo doesn't rename the other foo too -- can be improved!

declare const f3: <K extends string>(a: K, b: { [_ in K]: unknown }) => void

f3("foo", {
  foo: () => {}
})
// ctrl+click on b's foo, does nothing -- can be improved by navigating to a's foo!
// ctrl+click on a's foo, does nothing -- can be improved by showing b's foo as a reference!
// renaming one foo doesn't rename the other foo too -- can be improved!

πŸ“ƒ Motivating Example

A bunch of popular libraries that will benefit from this for example...

1. xstate

// @file machine.ts
import { createMachine } from "xstate"

export const postMachine = createMachine({
  initial: "idle",
  states: {
    idle: { entry: "doStuff" }
  }
})

// @file index.tsx
import { useMachine } from "@xstate/react"
import { postMachine } from "./machine"

const Post = () => {
  let machine = useMachine(postMachine, {
    actions: {
      doStuff: () => {
      
      }
    }
  })
}

This feature will enable ctrl+click on "doStuff" in index.tsx navigating to "doStuff" in machine.ts and renaming both "doStuff" by renaming either of them (given the types are correct ofc)

2. stitches

Taken from here

const Button = styled('button', {
  variants: {
    color: {
      violet: {
        backgroundColor: 'blueviolet',
        color: 'white',
        '&:hover': {
          backgroundColor: 'darkviolet',
        },
      },
      gray: {
        backgroundColor: 'gainsboro',
        '&:hover': {
          backgroundColor: 'lightgray',
        },
      },
    },
  },
});

() => <Button color="violet">Button</Button>;

Here the string literal "violet" in <Button color="violet">Button</Button> references the violet key in the object passed to styled function

3. framer-motion

Taken from here

import { motion } from "framer-motion"

const variants = {
  open: { opacity: 1, x: 0 },
  closed: { opacity: 0, x: "-100%" },
}

export const MyComponent = () => {
  const [isOpen, setIsOpen] = useState(false)

  return (
    <motion.nav
      animate={isOpen ? "open" : "closed"}
      variants={variants}
    >
      <Toggle onClick={() => setIsOpen(isOpen => !isOpen)} />
      <Items />
    </motion.nav>
  )
}

Here the string literals "open" and "close" in motion.nav's animate reference the keys of variants passed to it.

These three come to my mind right now, but I think this is a very common pattern so the improvement in the intellisense will have a significant impact.

@andrewbranch
Copy link
Member

I sort of thought this did work, at least in simple cases, but looks like we never consider string literals for rename/references with keyof? Super simple example:

interface A {
  a: string;
}

declare function f(a: A, k: keyof A): void;
declare let a: A;

f(a, "a");

I think I’d expect these to work, but wouldn’t be surprised if it’s complicated/expensive.

@andrewbranch andrewbranch added Bug A bug in TypeScript Effort: Difficult Good luck. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Aug 24, 2021
@andrewbranch
Copy link
Member

Labeled for Effort: Moderately Difficult?

@andrewbranch andrewbranch added this to the Backlog milestone Aug 24, 2021
@andrewbranch andrewbranch added the Domain: Refactorings e.g. extract to constant or function, rename symbol label Aug 24, 2021
@andrewbranch
Copy link
Member

FWIW, I think getting my example to work would be a win, and would probably fix the f2 test case. f3 may be more difficult, and a PR that doesn’t fix f3 would still be acceptable.

@devanshj
Copy link
Author

I was surprised you marked it as "bug" but then realized it indeed is a bug -- as the renaming refactor produces a compile error! xD

@andrewbranch
Copy link
Member

I’m calling it a bug because #5602 was marked as fixed. There are also #41489, #41923, and #41922 which are related.

paranoiacblack added a commit to paranoiacblack/TypeScript that referenced this issue Sep 9, 2021
    * Enables string literals in call expression to refer to property names.
    * Enables property names in call expression to refer to string literals.
    * Changes string literal rename/reference behavior when string literals are in a call expression.
      Previously, all string references with matching text would reference one another globally and
      allow renaming even if such an operation would result in a compiler error.

    Fixes microsoft#45549
paranoiacblack added a commit to paranoiacblack/TypeScript that referenced this issue Sep 9, 2021
    * Enables string literals in call expression to refer to property names.
    * Enables property names in call expression to refer to string literals.
    * Changes string literal rename/reference behavior when string literals are in a call expression.
      Previously, all string references with matching text would reference one another globally and
      allow renaming even if such an operation would result in a compiler error.

    Fixes microsoft#45549
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Refactorings e.g. extract to constant or function, rename symbol Effort: Difficult Good luck. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants