using data attributes token conversions break sometimes

[lifeiscontent]

Describe the issue

in this code:

text.stylex.ts

export const text = stylex.defineVars({
	fontWeight: stylex.types.integer(400),
});

component.tsx

const styles = stylex.create({
	div: {
		[text.fontWeight]: {
			":is([data-size='default'])": 500,
			":is([data-size='large'])": 600,
		},
	},
});

500 and 600 resolve to 500px and 600px

Expected behavior

I'd expect the value to remain intact

Steps to reproduce

create a component that applies the data property and see the styles resolve to 500px, 600px

Test case

do you guys have a sandbox you'd like me to put a repro together in?

I can do it in a follow up comment if so

Additional comments

No response

[srikrsna]

I could be wrong, but this could work:

const styles = stylex.create({
  div: {
    [text.fontWeight]: {
      ":is([data-size='default'])": stylex.types.integer(500),
      ":is([data-size='large'])": stylex.types.integer(600),
    },
  },
});

[nmn]

@srikrsna that might fix a type error (not sure if the TS types are even strict enough for that), but other than that stylex.types have no functional impact outside of defineVars.

create a component that applies the data property and see the styles resolve to 500px, 600px

Looks like the bug reported is that the values are being converted to px values instead of being maintained as numbers. Is this correct?

[lifeiscontent]

@nmn yeah, I'd expect if I used a type integer it would be maintained, or length would default to px in the scenario it was given a number.

[nmn]

@lifeiscontent That is a fair expectation, but to ensure that we can cache the result of compiling individual files, when you assign a variable, the compiler doesn't know what type the variable was initialised with.

I'm put up a PR that doesn't do any conversion for variables, so numbers will be maintained, as length values will need to be written as '10px' strings.

We should also update or create a new lint rule to enforce that variable values are always strings.

[lifeiscontent]

@nmn thanks for the context, that makes sense. 👍🏻