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

Not work for function #16

Open
vipcxj opened this issue Aug 13, 2019 · 11 comments
Open

Not work for function #16

vipcxj opened this issue Aug 13, 2019 · 11 comments
Labels
help wanted Extra attention is needed

Comments

@vipcxj
Copy link

vipcxj commented Aug 13, 2019

For example:

export const smooth = (fun: (offset: number) => number): (offset: number) => number => {
    return (offset: number): number => {
        const floor = Math.floor(offset);
        const ceil = Math.ceil(offset);
        if (floor === ceil) {
            return fun(offset);
        } else {
            return fun(floor) * (1 - offset + floor) + fun(ceil) * (offset - floor);
        }
    }
};

throw fun is not defined.

@ndelangen ndelangen added the help wanted Extra attention is needed label Sep 30, 2019
@oliviertassinari
Copy link

oliviertassinari commented Jun 6, 2021

I'm facing the same issue: https://codesandbox.io/s/floral-sea-bo4gk?file=/src/App.tsx. I noticed it in mui/material-ui#25677.

import { stringify, parse } from "telejson";
import { createTheme } from "@material-ui/core";

const stringified = stringify(createTheme());
const parsed = parse(stringified);

console.log("parsed", parsed.breakpoints.up("sm"));

export default function App() {
  return null;
}

Capture d’écran 2021-06-06 à 23 04 52

@andywhite37
Copy link

I'm having lots of issues with telejson and rendering Material UI v5 (React) components as well. It seems that many of the theme functions (like spacing, etc.) are being hijacked by something in storybook related to channel-pushmessage and telejson. I assume there is some wiring-up/hijacking of code when components are being rendered in iframes.

The problem is that the MUI theme functions are not serializable in the way that telejson is naively serializing them, so they can't be stringified, parsed and executed.

Is there any way to disable whatever is doing this in storybook? The components render fine on their own, but something is hijacking the function calls and messing things up.

@oliviertassinari
Copy link

I have been wondering if either we could solve this problem directly in the theme's level of Material-UI. I couldn't envision any. solution.

@andywhite37
Copy link

andywhite37 commented Jun 26, 2021

Yeah, I think the core issue is whatever is using telejson to try to serialize the theme, presumably to send the theme through an iframe postmessage. It appears that @react-theming/storybook-addon, is causing the issue to surface, but the root cause is likely a lower-level library. I haven't been able to figure out exactly where the theme is being serialized. I think it has something to do with the iframe channel postmessage stuff in storybook.

I don't think trying to modify the MUI theme to address this should be the fix - I think the problem is using a bad and seemingly unnecessary serialization of the theme in one of these addons.

I think you suggested this in another thread, but I think if the theme switcher can pass a serializable identifier through the iframe message (e.g. a theme name or index), rather than serializing the entire theme, that would be the best solution. I don't want to denigate this telejson library, but trying to serialize functions by stringifying the body and then trying to eval and execute the function is going to be a good approach - this only works for 100% pure functions that do not reference any values from outside their scope, and when it fails, it causes extremely hard to debug problems.

@tlmader
Copy link

tlmader commented Jul 12, 2021

@andywhite37 using @react-theming/storybook-addon, I was able to work around this by serializing/deserializing and recreating the theme:

/**
 * Serialize and deserialize theme to work around `values is not defined` error.
 */
const providerFn = ({ theme, children }) => {
  const serialTheme = JSON.parse(JSON.stringify(theme));
  const muiTheme = createTheme(serialTheme);
  return <ThemeProvider theme={muiTheme}>{children}</ThemeProvider>;
};

// create decorator
const themingDecorator = withThemes(null, [lightTheme, darkTheme], {
  providerFn,
});

export const decorators = [themingDecorator];

@Looky1173
Copy link

I've run into the same error and stumbled across this issue. Unlike others, I am using a custom createTheme() function with themeprovider-storybook therefore I can't use the aforementioned solution. Still, I managed to throw together something to work around the error but it is quite hacky and has a performance impact on my stories.

Is there any news on this? How is teleJSON essential for the workings of Storybook?

@austinmccalley
Copy link

I've run into the same error and stumbled across this issue. Unlike others, I am using a custom createTheme() function with themeprovider-storybook therefore I can't use the aforementioned solution. Still, I managed to throw together something to work around the error but it is quite hacky and has a performance impact on my stories.

Is there any news on this? How is teleJSON essential for the workings of Storybook?

@Looky1173 are you able to share this solution?

@andywhite37
Copy link

@tlmader - I think serializing the theme that way would only work if the theme only contains JSON-friendly values, like strings, numbers, booleans, array, etc. I don't think it would work for an MUI theme which contains functions in the theme itself.

@Looky1173
Copy link

Looky1173 commented Dec 28, 2021

@Looky1173 are you able to share this solution?

@austinmccalley I've scrapped my original hideous workaround and ended up doing nearly the same thing as @tlmader:

.storybook/preview.js

import { withThemesProvider } from 'themeprovider-storybook';
import { ThemeProvider } from 'styled-components';

const themes = []; // Your array of themes

/*
 * Serialize and deserialize theme to work around `values is not defined` error.
 * (Credit: https://github.com/storybookjs/telejson/issues/16#issuecomment-878359101)
*/
function customProvider({ theme, children }) {
    const serialTheme = JSON.parse(JSON.stringify(theme));
    const newTheme = customCreateThemeFunction(serialTheme); // This custom function returns a theme object
    return <ThemeProvider theme={newTheme}>{children}</ThemeProvider>;
}

export const decorators = [withThemesProvider(themes, { CustomThemeProvider: customProvider })];

Unfortunately it looks like this code executes on each rerender so it may slow down your storybook, especially if your (custom) ‘createTheme()’ function is expensive.

@austinmccalley
Copy link

Thanks for sharing your solution @Looky1173. It looks like MUIv5 support with Storybook is still limited and setting up a custom provider is still a little wonky right now. I won't be using Storybook until there is better support.

@Looky1173
Copy link

I don't think trying to modify the MUI theme to address this should be the fix - I think the problem is using a bad and seemingly unnecessary serialization of the theme in one of these addons.

I definitely agree! In fact, telejson is used in the core of Storybook as well. I think that this issue should be brought up in the main Storybook repository as it is more of an architectural problem. Sure, we could try implementing a dependency parser in telejson and include each dependency of the function serialized in the returned object, however, that would come with severe limitations and probably cause even more complications later on, as illustrated by this StackOverflow answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants