Merge master in feature/react-intl

[patsa]

Could someone please merge the current master in the feature branches (especially the react-intl branch)?
Currently a merge as recommended in https://github.com/kriasoft/react-starter-kit/blob/master/docs/recipes/how-to-integrate-react-intl.md throws some merge conflicts and I'm not really sure if the other changes are compatible with the current master, which is 104 commits ahead.

[piglovesyou]

Hi @patsa , yes that's a task. Since my time is limited, I'm so glad if someone makes a PR from the one's cloned

• master → feature/redux
• feature/redux → feature/apollo
• feature/apollo → feature/react-intl

, one by one.

[patsa]

I'll see if i have the time for it in the upcoming days. If the merge works, I'll let you know.

[patsa]

@langpavel @tim-soft @piglovesyou

I merged master in feature/redux (see PR #1877) and am currently in the process of merging feature/redux in feature/apollo. I think I managed to resolve all merge conflicts so far, but got stuck at the file /src/routes/home/Home.js.

Here is the initial conflict:

/**
 * React Starter Kit (https://www.reactstarterkit.com/)
 *
 * Copyright © 2014-present Kriasoft, LLC. All rights reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE.txt file in the root directory of this source tree.
 */

import useStyles from 'isomorphic-style-loader/useStyles';
import React from 'react';
import PropTypes from 'prop-types';
<<<<<<< HEAD
import { graphql, compose } from 'react-apollo';
import withStyles from 'isomorphic-style-loader/lib/withStyles';
import newsQuery from './news.graphql';
import s from './Home.css';

class Home extends React.Component {
  static propTypes = {
    data: PropTypes.shape({
      loading: PropTypes.bool.isRequired,
      news: PropTypes.arrayOf(
        PropTypes.shape({
          title: PropTypes.string.isRequired,
          link: PropTypes.string.isRequired,
          content: PropTypes.string,
        }),
      ),
    }).isRequired,
  };

  render() {
    const { data: { loading, reactjsGetAllNews } } = this.props;
    return (
      <div className={s.root}>
        <div className={s.container}>
          <h1>React.js News</h1>
          {loading
            ? 'Loading...'
            : reactjsGetAllNews.map(item => (
                <article key={item.link} className={s.newsItem}>
                  <h1 className={s.newsTitle}>
                    <a href={item.link}>{item.title}</a>
                  </h1>
                  <div
                    className={s.newsDesc}
                    // eslint-disable-next-line react/no-danger
                    dangerouslySetInnerHTML={{ __html: item.content }}
                  />
                </article>
              ))}
        </div>
=======
import s from './Home.css';

export default function Home({ news }) {
  useStyles(s);
  return (
    <div className={s.root}>
      <div className={s.container}>
        <h1>React.js News</h1>
        {news.map(item => (
          <article key={item.link} className={s.newsItem}>
            <h1 className={s.newsTitle}>
              <a href={item.link}>{item.title}</a>
            </h1>
            <div
              className={s.newsDesc}
              // eslint-disable-next-line react/no-danger
              dangerouslySetInnerHTML={{ __html: item.content }}
            />
          </article>
        ))}
>>>>>>> redux
      </div>
    </div>
  );
}

<<<<<<< HEAD
export default compose(withStyles(s), graphql(newsQuery))(Home);
=======
Home.propTypes = {
  news: PropTypes.arrayOf(
    PropTypes.shape({
      title: PropTypes.string.isRequired,
      link: PropTypes.string.isRequired,
      content: PropTypes.string,
    }),
  ).isRequired,
};
>>>>>>> redux

And here my solution to the conflict:

/**
 * React Starter Kit (https://www.reactstarterkit.com/)
 *
 * Copyright © 2014-present Kriasoft, LLC. All rights reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE.txt file in the root directory of this source tree.
 */

import useStyles from 'isomorphic-style-loader/useStyles';
import React from 'react';
import PropTypes from 'prop-types';
import { graphql, compose } from 'react-apollo';
import newsQuery from './news.graphql';
import s from './Home.css';

function Home({ data: { loading, reactjsGetAllNews } }) {
  useStyles(s);
  return (
    <div className={s.root}>
      <div className={s.container}>
        <h1>React.js News</h1>
        {loading
          ? 'Loading...'
          : reactjsGetAllNews.map(item => (
              <article key={item.link} className={s.newsItem}>
                <h1 className={s.newsTitle}>
                  <a href={item.link}>{item.title}</a>
                </h1>
                <div
                  className={s.newsDesc}
                  // eslint-disable-next-line react/no-danger
                  dangerouslySetInnerHTML={{ __html: item.content }}
                />
              </article>
            ))}
      </div>
    </div>
  );
}

Home.propTypes = {
  data: PropTypes.shape({
    loading: PropTypes.bool.isRequired,
    news: PropTypes.arrayOf(
      PropTypes.shape({
        title: PropTypes.string.isRequired,
        link: PropTypes.string.isRequired,
        content: PropTypes.string,
      }),
    ),
  }).isRequired,
};

export default compose(graphql(newsQuery))(Home);

But the variable reactjsGetAllNews is not accessible (ESLint: 'data.reactjsGetAllNews' is missing in props validation(react/prop-types)) and I honestly don't really know how this variable was accessible in the first place before the changes.
To make the merge work, I could just ignore the new functional component and use the old class structure, but I felt like that wouldn't make sense here.

Any ideas how to resolve the issue? If I can solve this, than I can make the PR for the merge.

Thanks in advance and best regards,
Patrick

[piglovesyou]

I cheer your marvelous work. I'd like to put suggestions on the issue you mentioned above.

• Manually declaring reactjsGetAllNews in Home.propTypes is one idea.
• Using React Hooks is another idea. This will change the feature/apollo a bit but will make the code simpler.

[patsa]

@piglovesyou
I'm currently back at it again, but have an error concerning the Apollo provider (probably).

TypeError

TypeError: this.client.watchQuery is not a function
    at Query.initializeQueryObservable (C:\Code\sauerborn_webforge\draupnir\node_modules\react-apollo\react-apollo.cjs.js:372:48)
    at new Query (C:\Code\sauerborn_webforge\draupnir\node_modules\react-apollo\react-apollo.cjs.js:283:15)
    at processChild (C:\Code\sauerborn_webforge\draupnir\node_modules\react-dom\cjs\react-dom-server.node.development.js:3159:14)
    at resolve (C:\Code\sauerborn_webforge\draupnir\node_modules\react-dom\cjs\react-dom-server.node.development.js:3124:5)
    at ReactDOMServerRenderer.render (C:\Code\sauerborn_webforge\draupnir\node_modules\react-dom\cjs\react-dom-server.node.development.js:3598:22)
    at ReactDOMServerRenderer.read (C:\Code\sauerborn_webforge\draupnir\node_modules\react-dom\cjs\react-dom-server.node.development.js:3536:29)
    at renderToStaticMarkup (C:\Code\sauerborn_webforge\draupnir\node_modules\react-dom\cjs\react-dom-server.node.development.js:4261:27)
    at process (C:\Code\sauerborn_webforge\draupnir\node_modules\react-apollo\react-apollo.cjs.js:1047:20)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at C:\Code\sauerborn_webforge\draupnir\src\server.js:195:1

Any ideas what could cause this?
By the way, this happens after a merge from feature/redux in feature/apollo.

Here is my current fork for replication purposes:
https://github.com/patsa/react-starter-kit/tree/feature/apollo

[langpavel]

@patsa did you upgrade packages? Can this introduce breaking change?

[patsa]

@langpavel
I only updated the changes from feature/redux.
So the changes in the lock file should be minor, except for those from feature/redux.

[langpavel]

@patsa I'm really sorry, but I and @piglovesyou welcome your help
What I can imagine: context is not well fed; or there is API incompatibility
Hope you will discover soon what's wrong

[patsa]

@langpavel @piglovesyou
Managed to fix the issue.

Here is my pull request:
#1881

[patsa]

@langpavel @piglovesyou
Hey everyone, I've come close to create a merge between my new feature/apollo and feature/react-intl.
I'm currently stuck with the following kind of errors on the 'Home'-page:

[React Intl] Missing message: "navigation.about" for locale: "en-US", using default message as fallback.
console.<computed> @ index.js:1
index.js:1 [React Intl] Missing message: "navigation.contact" for locale: "en-US", using default message as fallback.
console.<computed> @ index.js:1
index.js:1 [React Intl] Missing message: "navigation.login" for locale: "en-US", using default message as fallback.
console.<computed> @ index.js:1
index.js:1 [React Intl] Missing message: "navigation.separator.or" for locale: "en-US", using default message as fallback.
console.<computed> @ index.js:1
index.js:1 [React Intl] Missing message: "navigation.signup" for locale: "en-US", using default message as fallback.

Besides this error, I'm not able to switch the languages, because client is undefined within src/actions/intl.js, which leads to query not working and thus not being able to request the change.

Could someone please take a look at my repo on branch feature/react-intl and maybe help me out?

https://github.com/patsa/react-starter-kit/tree/feature/react-intl

Best regards,
Patrick

[patsa]

@langpavel @piglovesyou
Bump

[ulani]

@patsa thank you very much for crating this issue! Unfortunately, we have close it due to inactivity. Feel free to re-open it or join our Discord channel for discussion.

NOTE: The main branch has been updated with React Starter Kit v2, using JAM-style architecture.