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

Default Export extend Record<string, string> #71

Closed
nperez0111 opened this issue Jun 27, 2020 · 3 comments
Closed

Default Export extend Record<string, string> #71

nperez0111 opened this issue Jun 27, 2020 · 3 comments
Labels
good first issue Good for newcomers help wanted Looking for help to reproduce, debug, or open a PR

Comments

@nperez0111
Copy link
Contributor

Expected Behavior

I have a function that takes in a default export from a CSS Module but I want there to be a meaningful type for the parameter.

function foo( bar: Record<string, string> ): string[] {
	return Object.keys(bar);
}

Current Behavior

Export default looks like:

export interface Styles {
  'classname--first': string;
  'classname--second': string;
}

export type ClassNames = keyof Styles;

declare const styles: Styles;

export default styles;

Possible Solution

Export default could look like:

export interface Styles extends Record<string, string> {
  'classname--first': string;
  'classname--second': string;
}

export type ClassNames = keyof Styles;

declare const styles: Styles;

export default styles;

Context

If you have any suggestions on how to have a parameter that is meaningful for my function that would be really helpful but I think extending Record<string, string> allows for a lot.

Your Environment

  • Version used: Most Recent
  • Operating System and versions: macOS
  • Link to your project: Internal use
@skovy
Copy link
Owner

skovy commented Jul 3, 2020

Thanks for submitting the issue with all the details. I believe the recommendation would defeat the purpose of this package because it would then allow usage of any class (string) on the styles object. For example, this is now valid:

export interface Styles extends Record<string, string> {
  'classname--first': string;
  'classname--second': string;
}

export type ClassNames = keyof Styles;

const styles: Styles = {} as Styles;

styles.notARealClass;

However, I do see the issue with the code sample you provided. It looks possibly related to this issue: microsoft/TypeScript#15300. I believe if we updated to use a type instead of an interface this may fix your issue. I think we would have to consider this a breaking change because there could be subtle differences in how people are using the library. Would also love to know if there are other ways to handle your case! 🤔

type Styles = {
  'classname--first': string;
  'classname--second': string;
}

const styles: Styles = {} as Styles;

function foo( bar: Record<string, string> ): string[] {
  return Object.keys(bar);
}

foo(styles) // Valid with type Styles = {} but not interface Styles {}`

@skovy skovy added good first issue Good for newcomers help wanted Looking for help to reproduce, debug, or open a PR labels Jul 3, 2020
@nperez0111
Copy link
Contributor Author

I agree completely with your recommendation, it does seem that exporting as a type instead of an interface would resolve my issue. I’ll be able to make a PR for the update and any relevant documentation tomorrow.

Thank you for the support.

nperez0111 added a commit to nperez0111/typed-scss-modules that referenced this issue Jul 4, 2020
…y#71

Before we were unable to do this:
```typescript
export interface Styles {
  'classname--first': string;
  'classname--second': string;
}
function foo(bar: Record<string, string>){
	return Object.keys(bar);
}
```
Because an interface does not allow being cast as a wider type. This fixes that by using types instead of interfaces:
```typescript
export type Styles = {
  'classname--first': string;
  'classname--second': string;
}
```

BREAKING CHANGE: this can interfere with how others use their default exported classnames
@skovy skovy closed this as completed in 137a6eb Jul 6, 2020
@skovy
Copy link
Owner

skovy commented Jul 6, 2020

🎉 This issue has been resolved in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@skovy skovy added the released label Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Looking for help to reproduce, debug, or open a PR
Projects
None yet
Development

No branches or pull requests

2 participants