facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.34k stars 307 forks source link

TypeError: String.prototype.concat called on null or undefined (v0.6.1) #555

Closed sumanbh closed 3 months ago

sumanbh commented 5 months ago

Describe the issue

Seeing the following error on v0.6.1. Code was previously working on v0.6.0 and earlier versions

TypeError: String.prototype.concat called on null or undefined
    at concat (<anonymous>)
    at transformFile.next (<anonymous>)
    at run.next (<anonymous>)
    at transform.next (<anonymous>)

I was able to narrow it down to this new null check added in v0.6.1:

https://github.com/facebook/stylex/blob/c48bea12d3f4109a0b6bcce7c68c807972c16620/packages/babel-plugin/src/utils/evaluate-path.js#L693

In our case, we hit the following condition first (isStringLiteral is true) https://github.com/facebook/stylex/blob/c48bea12d3f4109a0b6bcce7c68c807972c16620/packages/babel-plugin/src/utils/evaluate-path.js#L683-L691

^ which defines func as the string concat function but context remains undefined.

As a result, calling func later results in TypeError: String.prototype.concat called on null or undefined being thrown because context is undefined. https://github.com/facebook/stylex/blob/c48bea12d3f4109a0b6bcce7c68c807972c16620/packages/babel-plugin/src/utils/evaluate-path.js#L725

I was able to workaround this by setting context to val. Reverting the null check also works but unsure which is the correct solution.

Expected behavior

No errors when compiling valid code.

Steps to reproduce

See issue description.

Test case

No response

Additional comments

No response

nmn commented 5 months ago

Could you share the code that is causing the error? The null check was needed for some other edge-cases we ran into.

adrian-novacescu commented 4 months ago

I have the same issue after updating. I updated from 0.4.1 to 0.6.1. I didn't try other versions. I can try to create a test case in 2 weeks if needed.

nmn commented 4 months ago

@adrian-novacescu Would you be willing to share a minimal repro? Ideally, start removing all the StyleX styles from your project until you find the root cause of the problem.

Thanks! (Sorry I was away for a couple of weeks)

adrian-novacescu commented 4 months ago

@nmn, I will try to do it until Friday. Worst case scenario I will do it during weekend.

adrian-novacescu commented 4 months ago

Unfortunately I didn't have the chance to work on it and the next couple of weeks seems to be extremely busy. Sorry for that. I will notify you as soon as I can do it :(.

nmn commented 4 months ago

@adrian-novacescu If you can even share the last few files where you used StyleX I might be able to find the root cause.

EvgeniyLyakhov commented 3 months ago

@nmn in my case code below throws same error when I use storybook

import * as stylex from "@stylexjs/stylex";

export const spacing = stylex.defineVars({
    3: '12px',
});

const styles = stylex.create({
    optionBase: {
        padding: `0 ${spacing['3']}`, // this line
    },
});
nmn commented 3 months ago

@EvgeniyLyakhov just to verify. But is spacing defined in a separate file with the .stylex.js file extension and imported from there when being used in the stylex.create?

adrian-novacescu commented 3 months ago

@nmn sorry it took me so long but I was full until now. I added a zip here that contain a test case where this can be reproduced. From what I see, it doesn't matter where the values are defined. As long as we use template literals for concatenating, it will fail. With normal string concatenation it works ok.

In the same test case I added another issue. I tried to use dynamic styles or variables for media queries but in this case I get: Only static values are allowed inside of a stylex.create() call.

All you need is to uncomment the code from index.tsx and run yarn production to reproduce that.

stylex.zip

nonzzz commented 3 months ago

According the minimal reproduction. I got the code before pipe to stylex compiler.

image AFAIK. Dynamic style only support arrow function expression. So cause the error translate. So i disable the @babel/preset-env in your local babel.config.js and i got the right code for stylex. image

@nmn Did you think it's a bug or a configuration error?

nonzzz commented 3 months ago

About StringLiteral being converted i'll take a look.

nonzzz commented 3 months ago

About babel configuration. I think we can add a document that describes its behavior to help users avoid incorrect configurations.

nmn commented 3 months ago

Thanks for understanding and then fixing this issue @nonzzz !