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, 12/1/2021 #46983

Closed
DanielRosenwasser opened this issue Dec 1, 2021 · 3 comments
Closed

Design Meeting Notes, 12/1/2021 #46983

DanielRosenwasser opened this issue Dec 1, 2021 · 3 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 1, 2021

Declare const enums as Non-const in Declaration Files

#46626 (comment)

  • const enums are supposed to be inlined, but isolatedModules issues an error because some compilers can't resolve across modules.
  • We also have preserveConstEnums - means you have a runtime representation, the compiler will inline the values, but downstream consumers technically can grab the values from runtime values.
  • Can be frustrating - inline is nice for libraries, but not for consumers because late binding (deferred resolution) is occasionally necessary across modules.
  • Idea was what if these flags changed the declaration file output?
    • Has issues for some "consumers" which are really the original authors. Separate compilations should be able to opt in to whatever they want.
  • Which flag modifis the behavior? isolatedModules or preserveConstEnum?
  • Another idea: preserve modifier.
  • Why is preserveConstEnum even a flag?
    • Version drift of the compiler, people would get enums with shifted versions when upgrading TypeScript slightly.
  • isolatedModules on a dependent project shouldn't have any bearing how a dependency should interpret thing.
  • Why do we have const enums again?
    • Huge performance gain thanks to just inlining the values.
    • Before modules and const existed.
    • Maybe JITs are better with const?
    • TDZ issues - not always easy, unclear if that's guaranteeed to be fast.
  • Only safe to inline when everything is "part of the same build process".
    • Whatever that means.
  • A preserved keyword could ask "is this part of a project reference" and do the right thing?
  • Hate the idea of another modifier though. This is really hard to explain.
  • Seems like there should be an inlining step from a "sufficiently smart bundler"™️
  • Reminds us of internal - who is a declaration internal to? Module? Project? Package? Monorepo/solution? Something else?
    • Maybe API Extractor gets to choose this.
  • Conclusion: not totally sold on the PR because we want to avoid adding more to const enums.

Partial Adds | undefined to Index Signatures in exactOptionalProperties

#46969

  • Generally speaking, when you apply Partial to a property, we add the missing type to the property.
  • What about index signatures?
    • Index signatures inherently are optional, so you can't mark them as optional.
    • All that would do for you is say whether or not you get undefined implicitly.
  • Could imagine it though - an index signature that implicitly reads undefined, but doesn't allow writes of it.
  • noUncheckedIndexAccess automatically adds the undefined.
  • So should we allow ? on index signatures?
    • Kind of acts as a way to migrate yourself onto noUncheckedIndexAccess.
    • But only if you're in exactOptionalProperties mode.
    • Otherwise, it just implicitly adds undefined to the index signature.
  • When a mapped type adds a ?, we add a regular undefined to the type, regardless of exactOptionalProperties.
  • If we were using exactOptionalProperties, we would probably use this in a few places.
  • Conclusion: feels like it can be valuable.

JSX Attribute Changes

Notes from @andrewbranch

facebook/jsx#132

(<div foo="hello"></div>)

Some people want to use template strings for everything, but the JSX spec doesn't currently allow it.

(<div foo=`hello`></div>)

Also proposing reinterpreting the character values of a string from the ECMA262 spec to the WHATWG spec for HTML attributes, meaning:

(<div foo="&nbsp;"></div>)

would be interpreted differently.

  • We would probably have to encode these string values as HTML internally
  • Should we provide feedback to Facebook on this?
    • There's a consistency problem:

      (<div title="&dash;">hello</div>)

      In HTML, this is a tooltip with text -. In JSX, it's currently a tooltip with text &dash;.
      Changing it would introduce a refactoring hazard if you change to:

      (<div title={"&dash;"}>hello</div>)
    • React could do this during rendering instead of the JSX transformer doing it; that would still be a big breaking change.

  • Conclusion: We don't have many details on this - need to follow up and clarify that we're understanding the proposal
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Dec 1, 2021
@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Dec 1, 2021

Some clarification on the JSX issue at facebook/jsx#132 (comment).

@fatcerberus
Copy link

fatcerberus commented Dec 2, 2021

Could imagine it though - an index signature that implicitly reads undefined, but doesn't allow writes of it.

I want this so much, you have no idea. noUncheckedIndexedAccess is a very large hammer and often makes dealing with arrays painful. Enabling it currently produces 200+ errors in one of my projects, many of which are spurious, and I’d rather leave it turned off than add non-null assertions everywhere (which are an even bigger hammer because they suppress future errors even if CFA/type checking gets more intelligent).

If we were using exactOptionalProperties, we would probably use this in a few places.

Indeed - exactOptionalProperties is enabled in the aforementioned project, and I really want my Record types to take advantage of it, without having to take the hit for array accesses as well. If it were opt-in for individual types, that’d be amazing.

@fatcerberus
Copy link

btw I actually just learned today that noUncheckedIndexedAccess also applies to foo.bar on an index signature type. Based on the name I had always assumed it only worked for foo["bar"] style accesses. foo.bar is not an “indexed access” according to the usual meaning of that term, so it’s a bit misleading.

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