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

toISOString is not a function #315

Closed
justusburger opened this issue Feb 27, 2017 · 20 comments
Closed

toISOString is not a function #315

justusburger opened this issue Feb 27, 2017 · 20 comments

Comments

@justusburger
Copy link

Since a few days ago, whenever I load my React + Redux app with the dev tools open, I get an error in the console saying:

Uncaught TypeError: toISOString is not a function
at String.toJSON ()

When I disable the Redux extension the error goes away. I suspect the issue has todo with the extension trying to parse/format my redux state and there is something that's causing issues.

The app I'm building is isomorphic, so I pass my store's initial state in a script tag. I would imagine that you'll be able to recreate the issue by simply trying to parse my initial state with the extension's parsing function.

Note that I do run the state.app section through immutable.fromJS on the client just before seeding the new store.

screen shot 2017-02-27 at 8 06 15 am

screen shot 2017-02-27 at 7 54 12 am

Im happy to send you the link to the site in question in a DM or something, just not 100% comfortable with posting it here.

@zalmoxisus
Copy link
Owner

zalmoxisus commented Feb 27, 2017

Most likely you're passing a React Synthetic Event, which cannot be serialized. See the similar issue and the troubleshooting. Note that the solution in that issue will not be available in React 16, since we're relying on React internals. So, I recommend to choose one of the solutions from the troubleshooting.

However, it would be great if we could make jsan not to throw in this case, but at least to skip those data. If you could add a pr with a test case there, it would be much appreciated. Seems like the issue with toISOString is only for specific events as I wasn't able to reproduce.

@zalmoxisus
Copy link
Owner

My guess is that React purges the events and the date's toISOString is not available. Since it's internal String.toJSON, it's difficult to say for sure what's happening. Maybe @kolodny has any other guesses.

@justusburger
Copy link
Author

justusburger commented Feb 27, 2017

Yeah you're right. I had some infrastructure that actually passed a React Component in the payload of an action. When I changed this to a new object with only the specific properties I was looking for, the issue when away.

So yes I believe the issue was actually the extension trying to parse a React Component.

This is an avoidable issue from the consumer point of view, but I believe it would be good to perhaps add some sort of check for this, as you suggested.

@justusburger
Copy link
Author

I know passing an React Component in an action sound like a bad idea and an anti pattern anyway, and I agree, but given the exact scenario it makes more sense.

Perhaps even just a check throwing an error whenever encountering a React Component anywhere in the state or any action saying it's a bad idea would also do the trick.

Great work on this extension btw. I wouldn't mind contributing should you guys need any more hands.

@zalmoxisus
Copy link
Owner

Not sure that storing React Components inside states is anti pattern. Seems there are some cases where it cannot be avoided. I'd like to support this (by restoring the reference), but it wouldn't be by default for performance reason. So, we anyway should find a solution here.

We should either find why jsan throws or wrap it in a try-catch and show a warning as was suggested in #227. A minimal repro case I could run would be appreciated.

@justusburger
Copy link
Author

I tried to whip up a quick example but did not get the same results. But basically it comes down to dispatching an action from a component's componentWillMount function passing "this" in the payload.

@kolodny
Copy link

kolodny commented Mar 3, 2017

Just to add to what @zalmoxisus was saying... I don't know much about synthetic events, however I do know that the toJSON() method of date objects do call toISOString. Here's a minimal example of that

d = new Date();
d.toISOString = null;
d.toJSON(); // Uncaught TypeError: toISOString is not a function

This explains what is happening (probably) but not why it's happening

@udnaan
Copy link

udnaan commented Mar 19, 2017

Same issue here.

Side-note: As a development tool, the extension throwing errors in the console is confusing. Perhaps that section that is throwing error should be in try-catch and the error itself should be confined to the extensions's own view panel.

@etse
Copy link

etse commented Apr 12, 2017

I got the same issue when using the extension together with redux-form. When I add new elements to the form with the window open this error is thrown.

@zalmoxisus
Copy link
Owner

@etse, could you please provide a repro so I'd find a way to bypass the exception. Maybe a pr modifying one of our examples?

zalmoxisus added a commit that referenced this issue May 4, 2017
@zalmoxisus
Copy link
Owner

zalmoxisus commented May 4, 2017

I wasn't able to reproduce the issue with redux-form demo. However I managed to address the case showed by @kolodny. It shouldn't throw in 2.15.0 (which was just published on Chrome Store). Not sure it will help because in that case the exceptions throws inside Date.toJSON, but according to the presented stack trace it should be from inside String.toJSON.

Neither React nor redux-form doesn't add custom toJSON functions, so I have no idea from where it could come, unless someone could provide a repro.

@daraclare
Copy link

daraclare commented Jun 14, 2017

I have the same issue using the Chrome extension version 2.15.0, any update on how to avoid this issue? Below is my configureStore file:

import { createStore } from 'redux';
import rootReducer from '../reducers';
import initialState from '../reducers/initialState';

export default function configureStore() {
  return createStore(
    rootReducer,
    initialState,
    window.__REDUX_DEVTOOLS_EXTENSION__ && window.__REDUX_DEVTOOLS_EXTENSION__()
  );
}

@zalmoxisus
Copy link
Owner

@daraclare it usually happens when you pass a React synthetic event as an action payload. See Troubleshooting for more information on how to solve this. Let me know if it helps.

@daraclare
Copy link

daraclare commented Jun 14, 2017

I have an onclick event here:

import React, { Component } from 'react';
import PropTypes from 'prop-types';
import './reduxpage.css';

export default class ReduxPage extends Component {
  render() {
    return (
      <div className="reduxpage">
        <h1>React & Redux Example</h1>
        <h1>{this.props.counter}</h1>
        <button onClick={this.props.actions.increment}> + </button>
        <button onClick={this.props.actions.decrement}> - </button>
      </div>
    );
  }
}

ReduxPage.propTypes = {
  counter: PropTypes.number.isRequired,
  actions: PropTypes.object.isRequired
};

but what I am making is a template to be used by other devs, so should I leave out the use of ReduxDevTools as the other developers will not know to change how they use synthetic events. If so, it seems like a shame, ReduxDevTools is so useful. Let me know what you think please?

@zalmoxisus
Copy link
Owner

You should use it like that:

export default class ReduxPage extends Component {
  increment() {
    this.props.actions.increment();
  }
  decrement() {
    this.props.actions.decrement();
  }
  render() {
    return (
      <div className="reduxpage">
        <h1>React & Redux Example</h1>
        <h1>{this.props.counter}</h1>
        <button onClick={this.increment}> + </button>
        <button onClick={this.decrement}> - </button>
      </div>
    );
  }
}

In this way there's no event payload sent to the actions. I know it looks cumbersome, but if there're lots of actions to call you can use one function which would call one of them depending on the event data.

@daraclare
Copy link

Thanks very much for the above, that was exactly the problem :)

@DZGoldman
Copy link

DZGoldman commented Jun 26, 2017

I was having the same issue as @justusburger - storing a React component in redux was throwing this error. The solution that worked for me was giving the React component in question a toJSON method that returns some custom string:

export class MyComponent extends Component {
  toJSON(){
    return "This will appear in redux dev tools"
  }
  render(){
    return(
      <div></div>
    )
  }
}

This overrides the default jsan.stringify behavior.

It would be nice if jsan could handle react components explicitly, and either actually stringify them or even just pass over them / return some safe, warning-indicator string, instead of throwing a app-breaking error.

Tried to throw together a minimal failing test, but haven't gotten it working yet.

@theghostbel
Copy link

We've removed event payload as suggested here by @zalmoxisus

           <div className="outward">
-            <button className="accept" onClick={onAcceptClick}>{'changes.accept'}</button>
+            <button className="accept" onClick={() => onAcceptClick()}>{'changes.accept'}</button>
           </div>

..and error is gone.

@mornhuang
Copy link

I comment the "import 'core-js/es6/date';" in my polyfils.ts, then everything is ok.
see zloirock/core-js#347

@zalmoxisus
Copy link
Owner

It should be handled in 2.16.5 which is available on Chrome Store.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants