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

await Promise.all fails to resolve due to optional chaining in JSX #52475

Closed
Fish1 opened this issue Jan 28, 2023 · 16 comments · Fixed by #52546
Closed

await Promise.all fails to resolve due to optional chaining in JSX #52475

Fish1 opened this issue Jan 28, 2023 · 16 comments · Fixed by #52546
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@Fish1
Copy link

Fish1 commented Jan 28, 2023

Bug Report - await Promise.all fails to resolve due to optional chaining inside JSX

I am sorry if this doesn't end up being a TypeScript bug, I'm just not able to replicate this without React at the moment. This feels more like a TypeScript issue? So I'll let you guys decide.

🔎 Search Terms

await Promise.all toString JSX ReactNode

🕗 Version & Regression Information

  • This is a type checking error
  • Broken on 4.9.4, works with version 4.6.2
  • I am only able to replicate this with React 17.0.2, not react 18

⏯ Repo Link

Sorry, I have to use a repo for this. It's a bit of a setup to create the conditions for this.
https://github.com/Fish1/bughunter

💻 Code

/src/pages/HomePage.tsx

import { ReactNode } from "react";
import MyList from "../component/MyList";

function HomePage() {

  async function myUnusedFunction() {
      const getDataFetch1 = Promise.resolve(["hello", "world"])
      const getDataFetch2 = Promise.resolve([{ id: 1 }, { id: 2 }])

      const [data1, data2] = await Promise.all([
        getDataFetch1, getDataFetch2 
      ]);

      // *******************************
      // TypeScript thinks data2 is a Promise<{ id: number; }[]>
      const x = data2.map((data) => {
        return data.id.toString();
      });
   
      // *******************************
      // TypeScript thinks data1 is a Promise<string[]>
      const y = data1.map((data) => {
        return data.toString();
      });

      return { x, y };
  }

  const myNodeArray: ReactNode[] = [
    <div>hello</div>,
    <div>world</div>,
    "hello",
    "sup",
  ]

  return (
    <div>
      <h1>Home Page</h1>
      <MyList columnsCurrent={myNodeArray}/>
    </div>
  );
}

export default HomePage;

/src/component/MyList.tsx

import { ReactNode } from "react";

export interface ListThingProps {
  columnsCurrent: ReactNode[],
}

function MyList(props: ListThingProps) {
  const { columnsCurrent } = props;

  return (
    <div>
      {
        columnsCurrent.map((column, index) => {

          /**
           * *******************************************************************
           * This variable existing will cause Promise.all() to not resolve its promises.
           * *******************************************************************
           */
          const brokenString = column?.toString();

          return (
            <div key={index}>
              {/* remove {column} here to fix the Promise.all() error */}
              {column}
            </div>
          )
        })
      }
    </div>
  );
}

export default MyList;

output of npx tsc

src/pages/HomePage.tsx:14:23 - error TS2339: Property 'map' does not exist on type 'Promise<{ id: number; }[]>'.

14       const x = data2.map((data) => {
                         ~~~

  src/pages/HomePage.tsx:14:23
    14       const x = data2.map((data) => {
                             ~~~
    Did you forget to use 'await'?

src/pages/HomePage.tsx:14:28 - error TS7006: Parameter 'data' implicitly has an 'any' type.

14       const x = data2.map((data) => {
                              ~~~~

src/pages/HomePage.tsx:18:23 - error TS2339: Property 'map' does not exist on type 'Promise<string[]>'.

18       const y = data1.map((data) => {
                         ~~~

  src/pages/HomePage.tsx:18:23
    18       const y = data1.map((data) => {
                             ~~~
    Did you forget to use 'await'?

src/pages/HomePage.tsx:18:28 - error TS7006: Parameter 'data' implicitly has an 'any' type.

18       const y = data1.map((data) => {
                              ~~~~


Found 4 errors in the same file, starting at: src/pages/HomePage.tsx:14

🙁 Actual behavior

await Promise.all doesn't resolve the promises.

🙂 Expected behavior

I expect Promise.all() to resolve it's types correctly regardless of a variable in another file.

Strange Solutions that I have come up with

  1. Comment out the brokenString variable in /src/component/MyList.tsx
  2. Uncomment the FixPromiseError() function in src/pages/DummyPage.tsx
  3. Remove {column} from inside the div in /src/component/MyList.tsx

Here is a link to the TypeScript community discord, and the thread with a lots more testing.
https://discord.com/channels/508357248330760243/1068268321998372964

Video of error (incase text description wasn't clear)

Video.webm
@Fish1
Copy link
Author

Fish1 commented Jan 28, 2023

Here is a demonstration of another "fix".

  1. create a "DummyPage"
  2. add a function that returns a promise
  3. import it before the home page is imported

This resolves all the issues with promise.all, It seemed like typescript didn't have the type definitions of the Promise library?

image

image

All fixed :D
image

@Fish1
Copy link
Author

Fish1 commented Jan 28, 2023

This error seems to be caused by the optional chaining of the ReactNode type.

The optional chain can exit if it has an if statement guard, and it is fine. It's only an issue when the code is depending on the optional chain.

Also, I can only seem to get this error when working with the ReactNode type in react.

image

@Fish1 Fish1 changed the title await Promise.all fails to resolve due a toString call inside JSX await Promise.all fails to resolve due a optional chaining in JSX Jan 28, 2023
@Fish1 Fish1 changed the title await Promise.all fails to resolve due a optional chaining in JSX await Promise.all fails to resolve due to optional chaining in JSX Jan 28, 2023
@Fish1
Copy link
Author

Fish1 commented Jan 28, 2023

Another test case: the optional chaining works fine, as long as you use the variable inside the JSX return of the map. And you don't use the column variable in any other pointless expression.

  1. create a string variable from column?.toString
  2. call column?.toString() (with or without the if guard) (with or without the optional chaining)
  3. put the myString variable inside the div

image
image
image

@Fish1
Copy link
Author

Fish1 commented Jan 28, 2023

Test Case: setting "allowJS": false inside tsconfig.json

If you set "allowJS": false in tsconfig.json, then VSCode will not detect the error. But TSC will continue to detect it.

image
image

@Fish1
Copy link
Author

Fish1 commented Jan 30, 2023

Test Case: Updating to React 18 fixes the issue. But I noticed the spot where they changed the ReactFragment type, and reverted it back to what was in React 17. This caused to issue to reappear.

@types/react/index.d.ts
image

Here I commented out the React18 ReactFragment, and brought back the React17 ReactFragment. And the issue shows up.

@types/react/index.d.ts
image

I then deleted the {} type from the Fragment, and the issue is resolved.

@RyanCavanaugh
Copy link
Member

Minimal repro I have so far:

import {} from "../component/MyList";

async function myUnusedFunction() {
  const getDataFetch1 = Promise.resolve(["hello", "world"])
  const [data1] = await Promise.all([getDataFetch1]);
  const y = data1.map((data) => {
    return data.toString();
  });
  return { y };
}
import { ReactNode } from "react";

function MyList() {
  [null as ReactNode].map(column => {
    column?.toLocaleString; // <-- key line
    const aa = column;
  });
}

Trying to simplify further but this is very, very delicate

@RyanCavanaugh
Copy link
Member

OK, so it's just these two files

import { ReactNode } from "react";

function MyList() {
  [null as ReactNode].map(column => {
    column?.toLocaleString; // <-- key line
    const aa = column; // <-- also key line
  });
}
async function myUnusedFunction() {
  const getDataFetch1 = Promise.resolve(["hello", "world"])
  const [data1] = await Promise.all([
    getDataFetch1 
  ]);
  data1.map(() => { });
}

listed in that order in tsconfig, with these dependencies:

  "dependencies": {
    "react": "17.0.2",
    "react-dom": "17.0.2"
  },
  "devDependencies": {
    "@types/react": "17.0.53",
    "@types/react-dom": "17.0.18",
    "@vitejs/plugin-react": "2.2.0",
    "typescript": "4.9.4",
    "vite": "3.2.5"
  }

I'm incredibly curious what's going on, but would really want the react dependency stubbed out into something that still repros this before investigating further. This bug, whatever it is, is very hard to hit.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jan 30, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 30, 2023
@Fish1
Copy link
Author

Fish1 commented Jan 31, 2023

@RyanCavanaugh Thank you so much! I think I was able to remove the React dependency. It is pushed to the "minimal" branch of my GitHub repo.

Branch: https://github.com/Fish1/bughunter/tree/minimal

/main.tsx

async function myUnusedFunction() {
  const fetch1 = Promise.resolve(['hello', 'world']);
  const [data1] = await Promise.all([fetch1]);
  const y = data1.map((data) => {
    return data.toString();
  });
  return { y };
}

export default myUnusedFunction;

/MyList.tsx

type myType = {} | null | undefined;

function MyList() {
  [null as myType].map(column => {
    column?.toLocaleString;
    const aa = column;
  });
}

export default MyList;

image

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 31, 2023
@Fish1
Copy link
Author

Fish1 commented Jan 31, 2023

Was able to take reduce the issue a little bit more. We can simply remove the map from the MyList file.

/main.ts

async function myUnusedFunction() {
  const fetch1 = Promise.resolve(['hello', 'world']);
  const [data1] = await Promise.all([fetch1]);
  data1.length;
}

/MyType.ts

type MyType = {} | null | undefined;

const myVar: MyType = null as MyType;

// you need the following two lines together, to make it work
myVar?.toLocaleString;
const x = myVar;

I also went and reset the tsconfig to what is produced by TSC init.

{
  "compilerOptions": {
    "target": "es2016",
    "module": "commonjs",                                
    "esModuleInterop": true,                            
    "forceConsistentCasingInFileNames": true,            
    "strict": true,                                     
    "skipLibCheck": true,                            
    "noEmit": true
  }
}

@Fish1
Copy link
Author

Fish1 commented Jan 31, 2023

Test Case: It appears to be that what matters is two expressions of myVar being used, where the first is a conditional chain.

image
image

@Fish1
Copy link
Author

Fish1 commented Jan 31, 2023

Was able to reproduce it in a single file, and have it in the playground.

type MyType = {} | null | undefined;

const myVar: MyType = null as MyType;

myVar?.toLocaleString;
myVar;

async function myUnusedFunction() {
  const fetch1 = Promise.resolve(['hello', 'world']);
  const [data1] = await Promise.all([fetch1]);
  data1.length;
}

https://www.typescriptlang.org/play?#code/C4TwDgpgBAsiAq5oF4oG8C+UA+UB2ArgDZE5QF4AmEAZgJZ4SUDcAUKwMYD2eAzsFAC2IAGoBDAE4AuWAiRRUhElDG9ZiSG1bDxEgPwA6YFwAyXDmKIQAysAkMA5mx2StqkHg5QaFDsDo8QiAAqoS8TABivv48ABQAlOisUFDcfAI0EMAcABYAjApQAAoSXIJ04QYSELxcRABuELEA2gDkORAkXK0ANFCtAO5cEkSUrQC68WwpafxQzZRiwGJ544ViA2J0AiVlFRAGlkQtmdn5k9NQi8t5BlZ4DsA5bBisQA

Seem like it was the mapping that wasn't allowing us to repro it in a single file.
image

@jakebailey
Copy link
Member

Since this reproduces, I did a bisect which points to #49119.

@jakebailey
Copy link
Member

The test, FWIW:

// @strict: true
// @target: esnext
// @lib: esnext

type MyType = {} | null | undefined;

const myVar: MyType = null as MyType;

myVar?.toLocaleString;
myVar;

async function myUnusedFunction() {
    const fetch1 = Promise.resolve(['hello', 'world']);
    const [data1] = await Promise.all([fetch1]);
    data1.length;
}

@RyanCavanaugh
Copy link
Member

@Fish1 big thanks for helping get to a short repro and @Andarist for the fast fix!

@Fish1
Copy link
Author

Fish1 commented Feb 1, 2023

Amazing work! Thanks everyone!

@Andarist
Copy link
Contributor

Andarist commented Feb 2, 2023

It was so bizarre that I couldn't resist debugging this 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants