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

Moving to version 3.0 #67

Closed
ryandrewjohnson opened this issue Apr 10, 2018 · 11 comments
Closed

Moving to version 3.0 #67

ryandrewjohnson opened this issue Apr 10, 2018 · 11 comments

Comments

@ryandrewjohnson
Copy link
Owner

ryandrewjohnson commented Apr 10, 2018

The next major release is in active development with the biggest update being the removal of the dependency on redux. By leveraging React's new Context API the library will now work without redux, while still providing the option to use redux for those users using it in their app.


Release plan:

release-candidate - READY!!!
✅ release v3.0.0


Todo for release:

✅ should work without redux by default
✅ should work with redux if store provided to LocalizeProvider
option to show default language on missing translation
✅ correct spelling for add language action #71
update documentation
✅ battle test new TypeScript definition #69
✅ update examples
document breaking changes
✅ provide migration guide


What's changed?

See official change log.

@ryandrewjohnson
Copy link
Owner Author

ryandrewjohnson commented Apr 26, 2018

🎉 Release candidate is ready!!! 🎉

For those that wish to try out v3.0.0:

npm install react-localize-redux@beta --save

To run the basic example, run the following from the root of /examples/basic directory:

npm install
npm start

If you are coming from v2.x.x a migration guide to go from v2 to v3 is in the works as well.

Lastly if you find any bugs please open an issue, and be sure to mention the release candidate version you are on.

@priyankmtr
Copy link

i am using "react-localize-redux": "^3.0.0-rc3". but i could not find <LocalizeProvider /> as mentioned in given office docs link. BTW, basic example link is also not working.

@ryandrewjohnson
Copy link
Owner Author

ryandrewjohnson commented May 24, 2018

@priyankmtr can you elaborate on what you mean by "I could not find" <LocalizeProvider />? I just installed the react-localize-redux": "^3.0.0-rc3 and ran the demo, and it appears to work fine.

Also I've updated the broken basic example link.

@priyankmtr
Copy link

Hi @ryandrewjohnson, Thanks for fixing the link. I got it now. It is working good. But when i am trying to include localeReducer in rootReducer, i am getting the issue, that i have shown in the attached image. Kindly pardon me if there is something wrong from my end. I am new to reactJS.

localeerror

package.json:-

  "devDependencies": {
    "@types/enzyme": "^3.1.10",
    "@types/enzyme-adapter-react-16": "^1.0.2",
    "@types/history": "4.6.0",
    "@types/jest": "^22.2.3",
    "@types/node": "^9.6.14",
    "@types/prop-types": "15.5.2",
    "@types/react": "^16.3.14",
    "@types/react-dom": "16.0.4",
    "@types/react-hot-loader": "^3.0.6",
    "@types/react-redux": "5.0.8",
    "@types/react-router": "^4.0.25",
    "@types/react-router-dom": "^4.2.6",
    "@types/redux": "^3.6.0",
    "@types/webpack-env": "1.13.0",
    "aspnet-webpack": "2.0.1",
    "aspnet-webpack-react": "3.0.0",
    "awesome-typescript-loader": "3.2.1",
    "bootstrap": "3.3.7",
    "css-loader": "0.28.4",
    "enzyme-adapter-react-16": "^1.1.1",
    "enzyme-to-json": "^3.3.3",
    "event-source-polyfill": "0.0.9",
    "extract-text-webpack-plugin": "2.1.2",
    "file-loader": "0.11.2",
    "identity-obj-proxy": "^3.0.0",
    "isomorphic-fetch": "2.2.1",
    "jest": "^22.4.3",
    "jest-trx-results-processor": "0.0.7",
    "jquery": "3.2.1",
    "json-loader": "0.5.4",
    "raf": "^3.4.0",
    "style-loader": "0.18.2",
    "ts-jest": "^22.4.5",
    "tslint": "^5.10.0",
    "tslint-microsoft-contrib": "^5.0.3",
    "tslint-react": "^3.6.0",
    "typescript": "^2.8.3",
    "url-loader": "0.5.9",
    "webpack": "2.5.1",
    "webpack-hot-middleware": "2.18.2"
  },
  "dependencies": {
    "@types/js-cookie": "^2.1.0",
    "@uifabric/icons": "5.7.1",
    "deep-object-diff": "^1.1.0",
    "enzyme": "3.3.0",
    "js-cookie": "^2.2.0",
    "office-ui-fabric-core": "9.6.0",
    "office-ui-fabric-react": "5.89.0",
    "react": "16.2.0",
    "react-dom": "16.2.0",
    "react-hot-loader": "3.1.3",
    "react-redux": "^5.0.7",
    "react-router-dom": "4.2.2",
    "redux": "^4.0.0",
    "typescript-string-operations": "^1.3.1"
  },

The difference between example and my application is, my application is using typescript. thanks.

@priyankmtr
Copy link

priyankmtr commented May 24, 2018

I tried to go with the way shown in the official docs.

import { LocalizeProvider } from 'react-localize-redux';

in my routes.tsx file. It is showing me error, that this module has no exported member 'LocalizeProvider'.

@thchia
Copy link
Collaborator

thchia commented May 25, 2018

@priyankmtr are you sure that you have installed the library? It doesn't show up in your dependencies, and if you use npm > v5 it should autosave.

@ryandrewjohnson
Copy link
Owner Author

@priyankmtr it looks like the type definition was missing for LocalizeProvider --- good catch. I don't actually use TypeScript in my project so maintaining the type definition is troublesome. If you find anything else please feel free to update the type definition and send a PR.

I have published an updated release candidate that can be installed like so:

npm install react-localize-redux@beta --save

@thchia
Copy link
Collaborator

thchia commented Jun 2, 2018

Quick question regarding the Translate component. I've noticed this logic regarding whether or not to add the default translation:

// Translate.js

constructor(props: TranslateProps) {
    super(props);

    this.state = {
      hasAddedDefaultTranslation: false
    };
  }

  componentDidMount() {
    this.setState({ hasAddedDefaultTranslation: true });
  }

  addDefaultTranslation(context: LocalizeContextProps) {
    if (this.state.hasAddedDefaultTranslation) {
      return;
    }
    ...
  }

If I use Translate like this (assume missing translations for both):

export default () => (
  {someCondition ?
    <Translate id='id_one'>DEFAULT 1</Translate> :
    <Translate id='id_two'>DEFAULT 2</Translate>
  }
)

addDefaultTranslate does not get called when someCondition changes. I think this is because during the React reconciliation process, the component instance is preserved, thus not flushing the state. Since the first default translation was added and state set accordingly, hasAddedDefaultTranslation remains true and the second default value is not set.

I can somewhat verify this by adding a key prop to Translate, and setting it equal to id. This forces React to see 2 different component instances, destroy the first Translate instance and reconstruct the second one, going through the entire component lifecycle again and correctly adding 'DEFAULT 2'.

I'm not sure how much of an edge case this is considered, or what the best solution is but my initial suggestion would be:

  1. Implement componentDidUpdate and check if the id prop has changed.
  2. If it has, reset state.hasAddedDefaultTranslation and call addDefaultTranslation again.

There is possibly another way using getDerivedStateFromProps (gDSfP) but I'm less familiar with that new API:

  1. Replace the call in componentDidMount with:
static getDerivedStateFromProps(nextProps, prevState) {
  if (prevState.lastKnownId === nextProps.id) {
    return null
  }
  return { hasAddedDefaultTranslation: false, lastKnownId: nextProps.id }
}

// need to save the last known id in state because you cannot access `this` in gDSfP.
  1. Inside addDefaultTranslation, set hasAddedDefaultTranslation to true.
  2. I think gDSfP runs before every render, so in the initial mount, when id goes from undefined to whatever value it has, addDefaultTranslation will run. It will then not run until the id prop changes, whereupon gDSfP resets the state flag and the subsequent render results in a call to addDefaultTranslation.

Happy to submit PR.

@ryandrewjohnson
Copy link
Owner Author

@thchia if you have time to do a PR with some tests covering the issue that would be much appreciated. I'm just getting things wrapped up with the v3.0 launch, so if you're able to tackle this before I release on Monday that'd be amazing.

@thchia
Copy link
Collaborator

thchia commented Jun 3, 2018

@ryandrewjohnson opened in #79. I appreciate you accommodating my contribution efforts 😄

@ryandrewjohnson
Copy link
Owner Author

v3.0.0 now released!!

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

No branches or pull requests

3 participants