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

Design Meeting Notes, 4/14/2021 #43743

Closed
DanielRosenwasser opened this issue Apr 20, 2021 · 0 comments
Closed

Design Meeting Notes, 4/14/2021 #43743

DanielRosenwasser opened this issue Apr 20, 2021 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

Keyword Bikeshed: OVERRIDE edition

#43660

  • Does it go before or after static?
  • docs.microsoft.com suggests static first in guidelines, but C# doesn't restrict it.
  • Between abstract and override, you'd want them in the same place, but there is no static abstract right now.
    • static is not part of the MethodDeclaration production.
  • Consensus: static precedes override, override comes after static.

Associating New Unreachable Aliases is Slow

#42284
#43622

  • When doing .d.ts emit, 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 Limitation bugs.
  • Conclusion: experiment with using the "more accessible" type.

Indirect Import Invocations Causes Perf Regressions

#35420
#35877
#43486

  • 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 correct this, they act differently.
  • Babel and others emit a (0, moduleNamespace.method)(...) instead of moduleNamespace.method(...).
    • We did this.
    • Win for compliance.
    • Loss in performance.
  • A few mitigations
    • One was to try to create reusable nodes for 0.
  • 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 allSettled gives you both in order to be available with something like SES.
  • 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. 🚲🏡
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

1 participant