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

4.8.2: Type serialization error with unique symbol #50720

Closed
jgoz opened this issue Sep 11, 2022 · 14 comments
Closed

4.8.2: Type serialization error with unique symbol #50720

jgoz opened this issue Sep 11, 2022 · 14 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@jgoz
Copy link

jgoz commented Sep 11, 2022

Bug Report

Original issue: stitchesjs/stitches#1078

When two types refer to the same unique symbol and they are combined via inference, exporting the inferred type results in a 4118 serialization error.

🔎 Search Terms

Unique symbol, serialized, error, 4118

🕗 Version & Regression Information

  • This changed between versions 4.7.4 and 4.8.2

⏯ Playground Link

// @filename: node_modules/@types/foo/symbols.d.ts
/** Unique symbol used to reference a property value. */
export declare const $$Sym: unique symbol;

/** Unique symbol used to reference a property value. */
export type $$Sym = typeof $$Sym;

// @filename: node_modules/@types/foo/properties.d.ts
export interface CSSProperties {
    a?: 'a';
    b?: 'b';
    c?: 'c';
}

// @filename: node_modules/@types/foo/stitches.d.ts
import type { $$Sym } from './symbols';
import type { CSSProperties } from './properties';

type ValueByPropertyName<PropertyName> = PropertyName extends keyof CSSProperties ? CSSProperties[PropertyName] : never

export type CSS<Utils = {}> =
    {
        [K in keyof Utils as K extends keyof CSSProperties ? never : K]?: Utils[K] extends (arg: infer P) => any
        ? (
            $$Sym extends keyof P
            ? (ValueByPropertyName<P[$$Sym]> | undefined)
            : never
        )
        : never
    }
    & {
        [K in keyof CSSProperties]: CSSProperties[K]
    }


export interface Stitches<Utils extends {} = {}> {
    css<
        Composers extends (string | { [name: string]: unknown })[],
        C = CSS<Utils>
    >(...composers: { [K in keyof Composers]: C }): unknown
}

export declare function createStitches<Utils extends {} = {}>(config?: { utils?: Utils }): Stitches<Utils>;

// @filename: node_modules/@types/foo/index.d.ts
import type { CSS } from './stitches';
export { createStitches } from './stitches';

import type { $$Sym } from './symbols';
import type { CSSProperties } from './properties';

export type PropertyValue<Property extends keyof CSSProperties, Config = null> = (
    Config extends null
    ? { readonly [K in $$Sym]: Property }
    : Config extends { [K: string]: any }
    ? CSS<Config['utils']>[Property]
    : never
)

// @filename: test.ts
// @showEmit: true
// @showEmittedFile: test.js
// @declaration: true
// @errors: 4118

import { createStitches } from 'foo';
import type * as Foo from 'foo';

export const { css } = createStitches({ // <- Error in 4.8.2 and above
    utils: {
        d: (value: Foo.PropertyValue<'c'>) => ({
            d: value
        }),
    }
})

const foo = css({
    c: 'c',
    d: 'c'
})

Workbench Repro

🙁 Actual behavior

Error: The type of this node cannot be serialized because its property '[$$Sym]' cannot be serialized.(4118)

🙂 Expected behavior

No error.

@jakebailey
Copy link
Member

@typescript-bot bisect good v4.7.4 bad v4.8.2

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 12, 2022

The change between v4.7.4 and v4.8.2 occurred at bf5acb5.

@jakebailey
Copy link
Member

jakebailey commented Sep 12, 2022

Bah, no, that isn't right. The bisecter instead bisected the build failing. (I'm not sure what it did, actually.)

@jakebailey
Copy link
Member

Okay, the bisect now points to #49221, which seems a lot more correct to me. See also #49065.

@jgoz
Copy link
Author

jgoz commented Sep 13, 2022

Thanks @jakebailey that looks directly related.

This is the workaround I used to solve the linked issue.

type WithPropertyValue<P> = {
  readonly [K in $$Sym]: P
}

export type PropertyValue<Property extends keyof CSSProperties, Config = null> = (
    Config extends null
    ? WithPropertyValue<Property>
    : Config extends { [K: string]: any }
    ? CSS<Config['utils']>[Property]
    : never
)

This avoids serializing the late-bound name ($$Sym) altogether because WithPropertyValue is rewritten as import('./path/to').WithPropertyValue<Property>.

If this is the only path forward, it seems like an acceptable tradeoff, especially given the complexity of the types in this library. I mostly wanted to validate that this was a deliberate change and not a regression.

@andrewbranch andrewbranch added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Sep 16, 2022
@andrewbranch
Copy link
Member

It seems like this is a known limitation, but I can’t bring myself to label it “working as intended.” It’s pretty weird and seems like we ought to be able to do better, but maybe it would take a lot of work.

@andrewbranch
Copy link
Member

I think it would at least depend on #44044, since IIUC it would have to synthesize an import declaration of $$Sym so it could access the value side of the symbol. But if we did #44044, this would probably become doable.

@testavenger
Copy link

When will this issue be fixed

@heathsnee

This comment was marked as spam.

@AlmogCHEQ
Copy link

Thanks @jakebailey that looks directly related.

This is the workaround I used to solve the linked issue.

type WithPropertyValue<P> = {
  readonly [K in $$Sym]: P
}

export type PropertyValue<Property extends keyof CSSProperties, Config = null> = (
    Config extends null
    ? WithPropertyValue<Property>
    : Config extends { [K: string]: any }
    ? CSS<Config['utils']>[Property]
    : never
)

This avoids serializing the late-bound name ($$Sym) altogether because WithPropertyValue is rewritten as import('./path/to').WithPropertyValue<Property>.

If this is the only path forward, it seems like an acceptable tradeoff, especially given the complexity of the types in this library. I mostly wanted to validate that this was a deliberate change and not a regression.

 can you please explain how to implement it?

@gajus
Copy link

gajus commented Dec 29, 2022

@AlmogCHEQ You need to patch stitches.

Something like this:

diff --git a/types/css-util.d.ts b/types/css-util.d.ts
index 1668fc2ab56c421894c547b2e5a988166cd90e5f..cfdad3f208ef7f57c89de7fb94cdaecbd7378c63 100644
--- a/types/css-util.d.ts
+++ b/types/css-util.d.ts
@@ -117,3 +117,11 @@ export type $$ScaleValue = typeof $$ScaleValue
 export declare const $$ThemeValue: unique symbol
 
 export type $$ThemeValue = typeof $$ThemeValue
+
+// https://github.com/microsoft/TypeScript/issues/37888#issuecomment-846638356
+export type WithPropertyValue<T> = {
+	readonly [K in $$PropertyValue]: T
+}
+export type WithScaleValue<T> = {
+	readonly [K in $$ScaleValue]: T;
+}
\ No newline at end of file
diff --git a/types/index.d.ts b/types/index.d.ts
index 8dbcc9cad3f6c556a3f370065dd95300a02dd973..dafadd22e8b6285aadee2630936a7918c9c5b02b 100644
--- a/types/index.d.ts
+++ b/types/index.d.ts
@@ -35,7 +35,7 @@ export type ComponentProps<Component> = Component extends ((...args: any[]) => a
 /** Returns a type that expects a value to be a kind of CSS property value. */
 export type PropertyValue<Property extends keyof CSSUtil.CSSProperties, Config = null> = (
 	Config extends null
-		? { readonly [K in CSSUtil.$$PropertyValue]: Property }
+		? CSSUtil.WithPropertyValue<Property>
 	: Config extends { [K: string]: any }
 		? CSSUtil.CSS<
 			Config['media'],
@@ -49,7 +49,7 @@ export type PropertyValue<Property extends keyof CSSUtil.CSSProperties, Config =
 /** Returns a type that expects a value to be a kind of theme scale value. */
 export type ScaleValue<Scale, Config = null> = (
 	Config extends null
-		? { readonly [K in CSSUtil.$$ScaleValue]: Scale }
+		? CSSUtil.WithScaleValue<Scale>
 	: Config extends { [K: string]: any }
 		? Scale extends keyof Config['theme']
 			? `$${string & keyof Config['theme'][Scale]}`

@artemis-prime
Copy link

artemis-prime commented Apr 30, 2023

Any word on this?? I'm seeing this highlighted in VSCode with TS 5.0.4 (in strict mode) and the latest stitches. Actually running tsc doesn't flag it though. (I'm telling vscode to use the project's version of ts, so that shouldn't be an issue).

@divmgl
Copy link

divmgl commented Jan 4, 2024

Currently running into this error now. Should we just not use symbols here?

@dzek69
Copy link

dzek69 commented May 7, 2024

To anyone fighting with this, here is a step-by-step solution:

stitchesjs/stitches#1078 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests