-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Closed
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Description
Refactor to Named Parameters
- "Named parameters"? what about "parameters object"?
- What about when there's a contextual type?
- Doesn't currently work, so we don't make it available.
- Lots of cases! Direct calls are simple, but then there's issues where you don't know if you can provide a good experience.
- None of our refactorings deal with this.
- Ben: not quite true, the
asyncrefactoring does have some sort of bailout case.
- Ben: not quite true, the
- Have to ask where we can offer this, and whether errors after refactoring is okay.
- Developers have mentioned that they don't trust refactorings that don't definitely have errors when something goes wrong.
- Could do a check to see whether the first parameter has any property overlap with the produced object argument.
- But that's useless in JS
- Could have different behavior for JS.
- Errors with things that definitely depend on positional parameters.
- We need to add something to the refactor API that explains why a refactor fails.
strictBindCallApplyis the only place where- Check whether or not all calls are detectable
- Overloads
- Not applied right now.
- Is that okay?
- Absolutely
- Overrides - when a subtype overrides a method in its base type.
-
Well it's the same problem as looking for all references of subtypes
-
Yeah, but you can always rename parameters
class C { foo(a: string, b: string): void; } class D extends C { // NOTICE: `c` and `d` have different names foo(c: string, d: string): void; }
-
- The fact that users can't apply the refactoring sometimes is actually worse than providing the refactoring and explaining why it won't work.
- [[5 minute bikeshed over whether or not we should provide an anonymous type for the annotation, or produce a local interface]]
- JSDoc
- There's an issue around renaming correctly (confirm @sanders_n)
- Paramter properties
- Don't provide the refactor here
- Rest parameters
- Doesn't work now, but could refactor the call to take an array of arguments (or tuple).
- Tuple?
- Yeah, just depends on the type of the rest parameter
- Maybe make the property optional entirely?
- That's a semantic change
- Just give it an empty array as an empty initializer
thisparameters- Just...just leave it alone.
- Parameters with initializers
- Default initializer in a binding pattern
- Becomes optional in the type
- All-optional parameters
- Makes the entire object argument optional
- What do you do in unannotated JS?
- Can't be sure! You have no validation when something goes wrong. You have no explicit intent. What if something's optional?
- Conclusion
es3.d.ts
- Why not?
- Yeah, but why not?
- First off, those people are targeting ES3 environments, they need some sort of break.
- Well even the compiler was restricted to ES3, and we could've made our lives easier by setting that as the
libinstead of yelling at them not to use.mapand.filterand.reduce
Omit in lib.d.ts
type Omit<T, U extends keyof T> = Pick<T, Exclude<keyof T, U>>;- We didn't want to conflict with other
Omits that had different behavior - and it would conflict with a global. - You're going to run into global scope problems anyway.
- But you might not, so why are you intentionally breaking people?
- But there are at least 2 different versions of
Omitthat are duplicated over DefinitelyTyped - Also, this uses
Excludeon a generic, so maybe you want to wait for negated types?
Loosening restriction on Array methods taking predicates
{} | null | undefinedis wrong, move tounknownfilternow allowsanywhich means you can not return- Seems like an error worth raising in the type system
- Why not switch
filterto - We need a bool-ish type
- What does that mean
- "I'm not so bool-ish on that change"
- This just needs
booleanorunknown, notany. - Seems like moving from
anytounknownis reasonable. findandfilterbeing different stinks.- Decide to try it out on RWC
Handling class fields breaking changes
Interesting ideas
- Add AST/checking support before we add
.d.tsemit support - Add
.jsemit support later on. - Maybe flip the name to
modernClassFieldEmit? - Can you make it depend on the target?
- Feels weird?
- Need a good way to evangelize the change if we do this, but the truth is that this is just slower and doesn't actually match our mental model of how the feature should work.
dragomirtitian, j-oliveras, karol-majewski, saschanaz, trotyl and 2 more
Metadata
Metadata
Assignees
Labels
Design NotesNotes from our design meetingsNotes from our design meetings