-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Closed
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Description
Keyword Bikeshed: OVERRIDE edition
- Does it go before or after
static? - docs.microsoft.com suggests
staticfirst in guidelines, but C# doesn't restrict it. - Between
abstractandoverride, you'd want them in the same place, but there is nostatic abstractright now.staticis not part of the MethodDeclaration production.
- Consensus:
staticprecedesoverride,overridecomes afterstatic.
Associating New Unreachable Aliases is Slow
- When doing
.d.tsemit, we start emitting the structural equivalent of a type if we don't have the alias available.-
A function that returns a local type, and has no return type annotation, we don't have a symbol that's accessible outside the function and we have to inline the structure of the type of a class.
function withSomething<T extends React.ComponentType<any>>(InnerComponent: C) { type ComponentProps = ElementConfig<C>; type Props = Omit<ComponentProps, keyof WithSomethingProps> return class WrappedComponent extends React.Component { render() { return <WrappedComponent> } } }
-
- Could imagine a heuristic of only re-aliasing if you have something that is more accessible.
- What is more accessible? Here it's obvious - local function alias is bad.
- Modules and globals - possibly weighted similarly. Would have to see.
- Separate issue about the types being incomplete - what's the deal with that?
- Well, we didn't entirely investigate that, but it's a recursive type. We don't really have the right machinery to model that.
- We actually could support more than one alias.
- Feels like you could maintain a stack of aliases.
- There is overhead associated with aliases.
- Every time you add a new alias, you have to keep around 3 other aliases which each could be waiting for instantiation. Have to either eagerly instantiate (up-front work) or hold around type mappers to instantiate (more memory).
- Also, lots of complexity.
- Could imagine a dual approach - back off in a more local scope, but in "equally weighted scopes", just don't do anything.
- If this were the only instance that we hit, we might have more clear consensus on holding only one type alias; but we've seen a few
Design Limitationbugs. - Conclusion: experiment with using the "more accessible" type.
Indirect Import Invocations Causes Perf Regressions
- CommonJS, AMD, UMD modules create an alias for an imported module and call methods off of it.
- Problem: some of these exported functions expect to be called with the correct
this; if they're not called with the correctthis, they act differently. - Babel and others emit a
(0, moduleNamespace.method)(...)instead ofmoduleNamespace.method(...).- We did this.
- Win for compliance.
- Loss in performance.
- A few mitigations
- One was to try to create reusable nodes for
0.
- One was to try to create reusable nodes for
- One idea: a flag.
--indirectImportInvocation?
- One strong opinion on not turning this on by default.
- Can't you just use the monkey-patching version off of
Promise.allSettled? Means you don't have to tell users to use a different build tool.- The shim that's uses for
allSettledgives you both in order to be available with something like SES.
- The shim that's uses for
- Why can't we add an additional helper variable for every call target?
- Live bindings
- Also, still more emit.
- Not big on more config on the intersection of people who need to polyfill AND use CommonJS AND need to target SES... but not totally against it.
- Conclusion: put it under a flag. What's the flag name?
- Make it longish, explicit on what it does.
- Not too long, it is a correctness fix.
- Bike shed. 🚲🏡
Metadata
Metadata
Assignees
Labels
Design NotesNotes from our design meetingsNotes from our design meetings