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

Improve parser error on mapped type with additional property declarations #45089

Closed
5 tasks done
AgainPsychoX opened this issue Jul 18, 2021 · 3 comments · Fixed by #46346
Closed
5 tasks done

Improve parser error on mapped type with additional property declarations #45089

AgainPsychoX opened this issue Jul 18, 2021 · 3 comments · Fixed by #46346
Assignees
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@AgainPsychoX
Copy link

AgainPsychoX commented Jul 18, 2021

Suggestion

Properties generators [Ed: Mapped types] require separate type definition. Is this really a requirement by design limitations? It's weird for me that I must to merge two types to get type I actually want. Also, depending on where the [foo in Foos]: Bar part is placed, it gives two different, but both hard to understand error - it would be nice to have easy to understand error at least, even if separate type is required.

🔍 Search Terms

Properties generation. Mapped types.

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Make properties generator thing work if it's amongst other properties. There should be check that generated property names are other than properties defined alongside.

If not possible (due to design limitations or something), make it return easy to understand error.

📃 Motivating Example

The 17th line } & { seems just a burden.

export type PricingSpans = {
    /// Pricing for day/hour if up to `days` days.
    [days: number]: number;

    /// Pricing for day/hour if more days than specified
    above: number;
}

export const PlaceTypes = ['openSky', 'roofed', 'garage'] as const;
export type PlaceType = (typeof PlaceTypes)[number];

type PricingInfo = {
	/// Pricing spans for various place types
    [placeType in PlaceType]: PricingSpans;

    // Removing next line makes it fail, weird...
} & {

    /// Pricing model
	model: 'hour' | 'day';
}

//------------------------------------------------------

const foo: PricingInfo = {
	openSky: {
		0: 40,
		above: 30
	},
	roofed: {
		0: 50,
		above: 40
	},
	garage: {
		0: 80,
		above: 70
	},
	model: 'day',
}
Output
export const PlaceTypes = ['openSky', 'roofed', 'garage'];
//------------------------------------------------------
const foo = {
    openSky: {
        0: 40,
        above: 30
    },
    roofed: {
        0: 50,
        above: 40
    },
    garage: {
        0: 80,
        above: 70
    },
    model: 'day',
};
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "target": "ES2017",
    "jsx": "react",
    "module": "ESNext",
    "moduleResolution": "node"
  }
}

Playground Link: Provided

Other examples:

  • Weird errors generated if the property generator part is placed after any other properties: Playground Link
  • Other weird error generated if the property generator part is placed first (before any other properties): Playground Link

💻 Use Cases

Its not important issue. Its just quality of life, something rarely used could look a bit better. Workaround is just separate the property generator and join with others. What's nice about the workaround is that it report error if properties generated overlap with properties defined attached (i.e, you can add 'model' into PlaceTypes, and there is proper error).

@MartinJohns
Copy link
Contributor

I have never heard the term "property generator" and was really confused, because I thought this was about generators. You're referring to mapped types.

@andrewbranch
Copy link
Member

I’m super open to improving the error message here (and also improving parser recovery 👀). The suggestion to allow declaring properties in mapped types is a duplicate of #13573, but that was closed in favor of #26797, which was closed by #44512, in which we ended up deciding not to allow index signatures for generic or non-infinite types, which sort of undermines the closure of #13573. But I think there are weird implications of mixing mapped types and property declarations that are elegantly solved with intersections, so we’re unlikely to go down that path at this point:

// Is this an error? What is `X1["two"]`?
type X1 = {
  [K in "one" | "two" | "three"]: string;
  two: number;
};

// If the above is an error, this must be an error, right?
type X2<T> = {
  [K in keyof T]: string;
  two: number;
};

// Because if it were not an error, this would be an error?
type X3 = X2<{ two: any }>;

// And we don’t like errors on instantiation without some explicit constraint on the type parameter :(

@andrewbranch andrewbranch changed the title Properties generator require separate type definition if there are other properties Improve error message on mapped type with additional property declarations (bad parser error) Jul 21, 2021
@andrewbranch andrewbranch added Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript labels Jul 21, 2021
@andrewbranch andrewbranch added this to the TypeScript 4.5.0 milestone Jul 21, 2021
@sandersn sandersn changed the title Improve error message on mapped type with additional property declarations (bad parser error) Improve parser error on mapped type with additional property declarations Oct 11, 2021
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Oct 13, 2021
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Oct 13, 2021

The dual to this issue is #18299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants