Skip to content

[NewErrors] 4.6.0-dev.20211126 vs 4.5.2 #46939

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

Closed
typescript-bot opened this issue Nov 28, 2021 · 3 comments
Closed

[NewErrors] 4.6.0-dev.20211126 vs 4.5.2 #46939

typescript-bot opened this issue Nov 28, 2021 · 3 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@typescript-bot
Copy link
Collaborator

The following errors were reported by 4.6.0-dev.20211126, but not by 4.5.2

lensapp/lens

4 of 5 projects failed to build with the old tsc

tsconfig.json

@RyanCavanaugh
Copy link
Member

I'm not 100% sure why this wasn't an error before. The loop definitely exists:

export class Wizard extends React.Component<WizardProps, State> {
  public state: State = {
    step: this.getValidStep(this.props.step), // <----
  };

  get steps() {
    const { className, title, step, header, onChange, children, ...commonProps } = this.props;
    const steps = React.Children.toArray(children) as WizardStepElem[];

    return steps.filter(step => !step.props.skip).map((stepElem, i) => {
      const stepProps = stepElem.props;

      return React.cloneElement(stepElem, {
        step: i + 1,
        wizard: this,
        next: this.nextStep,
        prev: this.prevStep,
        first: this.firstStep,
        last: this.lastStep,
        isFirst: this.isFirstStep, // <------
        isLast: this.isLastStep,
        ...commonProps,
        ...stepProps,
      } as WizardStepProps<any>);
    });
  }
  get step() {
    return this.state.step; <----
  }
  isFirstStep = () => this.step === 1; <-----

  protected getValidStep(step: number) {
    return Math.min(Math.max(1, step), this.steps.length) || 1; <----
  }

need to bisect this based on an isolated repro to see if the root cause is problematic

@RyanCavanaugh RyanCavanaugh self-assigned this Nov 29, 2021
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Nov 29, 2021
@RyanCavanaugh
Copy link
Member

OK, here's the tiny version

declare const props: WizardStepProps;
export class Wizard {
  get steps() {
    return {
      wizard: this,
      ...props,
    } as WizardStepProps;
  }
}

export interface WizardStepProps {
  wizard?: Wizard;
}

@RyanCavanaugh
Copy link
Member

Caused by #45729. When we do subtype reduction of the two contributors of wizard in the spread, which are this and Wizard. A logical workaround given those facts would be to write

    return {
      wizard: this as Wizard,
      ...props,
    } as WizardStepProps;

but this also fails in main today. I have a fix in removeSubtypes that "works" by adding an optimization to bail early on 1 types, which is troubling.

RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Dec 1, 2021
RyanCavanaugh added a commit to RyanCavanaugh/lens that referenced this issue Dec 1, 2021
This code has a type circularity that previously went undetected. Adding an `as Wizard` assertion here removes the circularity.

See microsoft/TypeScript#46939 / microsoft/TypeScript#46981
RyanCavanaugh added a commit that referenced this issue Mar 8, 2022
…ircularity problems (#46981)

* Creates a reasonable workaround for #46939

* Remove unrelated newline

* Do the cheap length check ahead of the cache check

* Actually do it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

2 participants