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, 8/2/2024 #59524

Open
DanielRosenwasser opened this issue Aug 2, 2024 · 0 comments
Open

Design Meeting Notes, 8/2/2024 #59524

DanielRosenwasser opened this issue Aug 2, 2024 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

Rethinking BuiltinIterator and Introducing IteratorObject

#59506

  • Created two related but separate new features in 5.6
    • --strictBuiltinIteratorReturn
    • Iterator helper methods, which are installed on BuiltinIterators.
  • Annoyingly, you always have to write BuiltinIterator<T, BuiltinIteratorReturn> instead of just BuiltinIterator<T> - over and over.
    • You'd assume it could be the default - but you can't because generators inherit from BuiltinIterator and they default to void as the TReturn.
      • Not compatible.
  • What's proposed?
    • The type representing values backed by Iterator.prototype - is renamed to IteratorObject<T, TReturn, TNext>.
    • BuiltinIterator still exists, but now it's now just BuiltinIterator<T>. It inherits from IteratorObject<T, BuiltinIteratorReturn, unknown>.
    • All methods returning Iterator.prototype-backed iterators still return a BuiltinIterator<T>.
  • Do we need compatibility between the beta and RC?
    • No, because things aren't stable yet.
  • We could also have separate ArrayIterator<T>, MapIterator<T>, and SetIterator<T> types, but that's not in the PR.
    • Should we?
      • It could avoid confusing people trying to decide between IteratorObject and BuiltinIterator.
        • Might still need to write an explicit IteratorObject<SomeType, BuiltinIteratorReturn, unknown> in a lot of cases.
          • DOM types might actually have their own iterator type too. Have to check.
      • These all have different runtime types, so it might be a good idea. If someone monkey-patches one of these, it shouldn't appear on other iterators.
  • Reminder: what is BuiltinIteratorReturn?
    • An intrinsic type that represents the return type of a BuiltinIterator. Either any or undefined depending on --strictBuiltinIteratorReturn (which is under --strict).
    • Probably needs a comment in lib.d.ts.
  • Should IteratorObject have defaults of unknown for TReturn/TNext?
    • It would be nice! Iterator<T> doesn't need TReturn and TNext.
    • Should TNext actually be something more specific than unknown?
      • Possibly fine - it's all about delegation.
    • Leaning towards doing this.

Iterator Helpers for ES5-Emitted Generators

#59514

  • A way of correctly linking up polyfills of Iterator.prototype with downleveled generators.
  • Does this work with ES6 generators?
    • Yes, the iterators themselves exist and you can polyfill the behavior.

Refactoring NodeBuilderFlags

#59440

  • We have too many flags, can't be represented within 32 bits.
  • Can try to avoid this by adding another parameter, using an object, possibly a bigint.
    • Don't waste time on bigint, it will never be efficient.
  • Why not just parameters?
    • Need an overload (it's in the public API and there are arguments that follow it)
    • Also: want to avoid flag confusion.
  • How many public APIs take this?
    • Like 10 publicly.
  • Also, we have a bunch of internal flags that are just for us.
    • Maybe start by moving all the internal flags into a separate enum, keep the user-facing ones on the existing public one.
    • Can just add internal overloads instead.
  • People prefer two different enums, will figure out how the overloads should look.

"Fail Fast" Flag For --build (a.k.a. --noDownStreamOnError)

#59433

  • Previously, as soon as a project had errors under --build, TypeScript would just stop. Wouldn't emit files, wouldn't check other projects.
  • In 5.6 this changed. Nice because it allows flexibility when a codebase is in an error state. Upgrades can happen incrementally in other parts of the codebase.
  • Kind of annoying in a different way. Going project-by-project from upstream (the roots) to downstream (the leaves) is much harder because TypeScript blindly keeps building every depending project.
    • Some of us thought you could opt into the old behavior if you write noEmitOnError - but that's not the case. It would toggle TypeScript not to produce outputs, but it will still continue to check other projects.
  • So the PR introduces a flag to "fail fast" on a project error.
  • Currently named noDownstreamOnError, but not a fan of making users think about the words "upstream"/"downstream".
    • What should we name the flag?
    • we like --stopBuildOnErrors.
  • This new flag almost implies the old behavior - but not exactly the same.
    • How?
      • Previously, a failing project wouldn't produce files and "failed fast" on the entire build. Under this mode, the failing project will still produce files but will fail fast after that.
      • [[Note: Possible that --noEmitOnError in conjunction with the new flag is basically what you want?]]
  • Kind of odd that the flag is opt-in. Old behavior might be more desirable by default.
  • Should the two flags have some implication of the others?
    • Maybe?
  • We want to revisit to think about the flow here.
@RyanCavanaugh RyanCavanaugh added the Design Notes Notes from our design meetings label Aug 2, 2024
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

2 participants