Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

React Support for client side #6044

Open
deepu105 opened this Issue Jul 6, 2017 · 40 comments

Comments

Projects
None yet
Member

deepu105 commented Jul 6, 2017 edited

This is the central ticket for our React support.
This will be an evolving ticket, I'll create a Project once we have identified TODO items clearly
Any TODO must be added/modified to this opening comment.

The below is the Framework choices made so far:

  • React written in Typescript
  • Redux + react-redux + Axios + Promise middleware + redux-thunk
  • React router v4
  • Bootstrap 4 + Reactstrap
  • Tslint + tslint-eslint + tslint-react
  • Lodash
  • Karma + Mocha + Chai + Enzyme for unit tests
  • Sass support
  • i18n with counterpart + react-translate-component
  • Weback set up close to our Angular setup

The below are the initial list of TODO:

  • Merge the generator-jhipster-react library into a new branch in the generator-jhipster repo (@deepu105)
  • Migrate to bootstrap from material UI (@sendilkumarn @deepu105)
  • Migrate to Typescript from ES6 + babel (@sendilkumarn @deepu105)
  • Add options to tempate for AUth, Sass, i18n etc
  • Find a solution for i18n so that our current files can be reused. If cannot find a lib write a plugin using counterpart adapting ngx-translate(@deepu105)
  • Cleanup structure and finish almost done pages if any (@deepu105)
  • Add sample unit tests
  • Migrate account module pages
  • Migrate entity sub generator

cc @jhipster/developers

Member

atomfrede commented Jul 6, 2017

We are currently using React and Vue.js and are exploring an approach to write custom components once with preact and then use a thin wrapper to use the component both in react (or just use the preact version) and in vue. Maybe we should also think about something similar to make it possible to use another frontend if required (or for a module that provides vue instead of react).

Member

deepu105 commented Jul 6, 2017

In my personal experience using 2 frameworks whuch does same job only adds complexity and makes maintenance harder and implementation flaky. I would rather stick 2 just one framework. Evn using angular 1 and 2 together needed a lot of duct tape. So I would recommend to keep Vue out of question for now.

From point of Jhipster, what would be more beneficial would be to find a way to add alternative options simpler

Member

atomfrede commented Jul 6, 2017

Absolutely! Supporting two frameworks in core generator is hell. When we find a way to write components (e.g. table component) once (in preact) and can use that component in different ui frameworks with little overhead (preact is 3kb) this would at least help to be able to add additional options (either in the main generator or as a module) as we would not need to rewrite everything in the new fraemwork, but agree, lets focus on react first.

Member

deepu105 commented Jul 7, 2017

@sendilkumarn I see that you assigned items 2 & 3 to yourself, I have already done this on my company project which is based on JHipster-react so I'll do the base changes and then you can work on top of that.

Contributor

sendilkumarn commented Jul 7, 2017

Okay 👍

Contributor

erikkemperman commented Jul 18, 2017 edited

Just wondering -- from the current package.json in this branch it seems this would be adopting FaceBook's react implementation, is that right? Have you guys considered the legal implications, given the controversial BSD+PATENTS attached to it? I am not a lawyer, but there's quite a bit of unease around their license. See, e.g.,
https://news.ycombinator.com/item?id=14779881
https://react-etc.net/entry/apache-foundation-bans-use-of-facebook-bsd-patents-licensed-libraries-like-react-js
Maybe it would be better to look at alternative implementations?

EDIT: Maybe keep an eye on this -- FB are aware that this is scaring a lot of folks: facebook/react#10191

Member

deepu105 commented Jul 18, 2017 edited

@erikkemperman I'm aware of it. Technically we are not an Apache Foundation project and hence this doesn't affect us. I do understand the BDS + Patents license is slightly less permissive than APL2 (I'm not a lawyer) But in any case this shouldn't worry us, in the rarest ever possibility that someone encounters a legal issue becoz of the React licence, Never happened so far, it would be very very easy to switch to something like Preact or Inferno (which are 100% API compatible) which are MIT licenced. SO let's not worry much about this now. Also once React support is added it's too easy to add an option to choose between React or Preact so for me this is not an issue at all.

Contributor

erikkemperman commented Jul 18, 2017

@deepu105 Thanks for the quick response Deepu!

Yes, I know JH is not an ASF project, I was just curious if their reasoning would apply here as well. Apparently the fear is, roughly, that using React in your projects might allow FaceBook to co-opt all of your patents, related or not, because you give up the right to sue them for infringement.

It's probably not actually that bad, but still... ASF doesn't drop this bombshell lightly I imagine.

I just wanted to make sure this concern is on the radar, that is all.

Owner

jdubois commented Jul 18, 2017

@erikkemperman yes of course, in fact I tweeted about the legal implication about 1 year ago, and since then I'm regularly attacked by React developers in France.

For me we have no issue at all here, but this needs a long explanation.

First let me detail the issue with React:

  • There are legal implications for people using React. Let's take the original example I used one year ago: you develop a software for a client, using React. That client gets bought by another company. That other company sues Facebook - and hence loses the right to use your software.
  • There are a lot of attacks/trolls/insults from the React community, and also a FAQ that tells there is no problem whatsoever. Just read my example above, it has been correct from the start, and it is still correct. Now I don't care being insulted by people with no legal background.
  • As it is often replied: that company suing Facebook is "bad". Yes, I agree, and that's why I did this specific example. It's not the software developer's role to tell that the company that is buying his client is bad, and "punish" them. Who do you think you are, Batman?
  • The whole point of OSS is about the license, and not strictly following the license is wrong from my point of view. Younger people probably don't know about this, but this kind of use case already happened in the past, for example there was one person who did a GPL project, and added a clause to forbid the South African police to use it. Same thing as my previous point: we're here for making software, we're not Batman.

Now let me detail the issues we have with the use of React in JHipster:

  • We're not directly using React: we generate an application that uses React. And anyway, we're not even a company, so I wonder which kind of issue we could have (and I'm not planning to sue Facebook, anyway). I think that on our side, we are safe. And in case of trouble, we'll just drop the React option, or put it inside a module. So no problem at all for the project itself.
  • There is indeed something that worries me: this means JHipster could not become an ASF project one day. This question arises from time to time, at the moment we have no plans of doing this - basically this is because they rise a lot of legal issues and paperwork, and nobody is ready to do that (I just don't have that much free time).
  • The real issue is for our users: but it's like any JHipster-generated project, that project is the user's project, and that's his own issue. Maybe he thinks that's fine for him, maybe his legal team says it's fine - anyway that's his project and he does whatever he wants with it. We already support Oracle and MS SQL Server, which are totally proprietary technologies - honestly the React clause is a total non-issue for us, at this point.
Contributor

erikkemperman commented Jul 18, 2017

I was just thinking of the users, actually -- myself included :-) Thanks for detailed response, good to hear you guys have thought about this.

Member

deepu105 commented Jul 18, 2017

@jdubois we are anyway safe even for your last 3 points, If we encounter issue or if we go ASF we will just switch React with Preact which has MIT license but works the same way as React and works with all other libs, so the only code change would be to change the imports technically (It can be done by find replace) look at this for example ui-router/visualizer@f07f09c

Contributor

sendilkumarn commented Jul 18, 2017

Instead of switching to preact, why can't we start with preact 😜

@jdubois really detailed and well laid out response 👍 Thanks 😄

Hello, little question. Why not use Material-ui ? I was planning to use it for Desktop application, and switch to material-ui-native for mobile (when possible). Could it be possible to have the choice in the command line ?

Contributor

sendilkumarn commented Jul 18, 2017

@survivant we want to align more towards our current JHipster Angular UI which is based on Bootstrap.
Adding a choice to choose between material UI and bootstrap will make the project more cumbersome. So definitely no for now. May be you can add in a module for that.

@sendilkumarn make sense, I'll have to check how to create a module. Could be more complete that way, if I want to generate code for web and native applications.

Member

deepu105 commented Jul 18, 2017

@sendilkumarn React is more popular and has more reach so lets start with it first, we will see about issue if we ever encounter one

survivant commented Jul 18, 2017 edited

@deepu105 I'll like to play around with this branch. How can I generate app with the code in the branch ? I try to follow the documentation on : https://github.com/hipster-labs/generator-jhipster-react

but I obtain this error message

$ yo jhipster-react
Error jhipster-react

You don't seem to have a generator with the name “jhipster-react” installed.
But help is on the way:

You can see available generators via npm search yeoman-generator or via http://yeoman.io/generators/.
Install them with npm install generator-jhipster-react.

To see all your installed generators run yo without any arguments. Adding the --help option will also show subgenerators.

If yo cannot find the generator, run yo doctor to troubleshoot your system.

I'm not too familiar with npm

thanks, I hope to be able to help to test

Contributor

gmarziou commented Jul 19, 2017 edited

@survivant, these instructions are obsolete as react support is no longer a module but a sub generator of main generator.
Now you can follow https://github.com/jhipster/generator-jhipster/blob/jh-react/CONTRIBUTING.md#setup.

So it should be:

git clone https://github.com/jhipster/generator-jhipster
cd  generator-jhipster
git checkout jh-react
yarn install
yarn link

And to use in an app:

mkdir my_react_app
cd my_react_app
yarn link generator-jhipster
yo jhipster

survivant commented Jul 19, 2017 edited by gmarziou

@gmarziou thanks. I'll start from there.

If I found issues, should I start to flag them ?

for now, it's not working. here the output
`C:\Data\workspace_perso\github-forks\jhipster-react-branch>yo jhipster

Welcome to the JHipster Generator v4.6.0
Documentation for creating an application: https://jhipster.github.io/creating-a
n-app/
Application files will be generated in folder: C:\Data\workspace_perso\github-fo
rks\jhipster-react-branch
WARNING! Failed to connect to "git://github.com"
1. Check your Internet connection.
2. If you are using an HTTP proxy, try this command: git co
nfig --global url."https://".insteadOf git://


JHipster update available: 4.6.2 (current: 4.6.0)

Run yarn global upgrade generator-jhipster to update.


? (1/16) Which type of application would you like to create? Monolithic appli
cation (recommended for simple projects)
? (2/16) What is the base name of your application? jhipster
? (3/16) What is your default Java package name? com.mycompany.myapp
? (4/16) Do you want to use the JHipster Registry to configure, monitor and sca
le your application? No
? (5/16) Which type of authentication would you like to use? JWT authenticati
on (stateless, with a token)
? (6/16) Which type of database would you like to use? SQL (H2, MySQL, MariaD
B, PostgreSQL, Oracle, MSSQL)
? (7/16) Which production database would you like to use? MySQL
? (8/16) Which development database would you like to use? H2 with disk-based
persistence
? (9/16) Do you want to use Hibernate 2nd level cache? No
? (10/16) Would you like to use Maven or Gradle for building the backend? Maven

? (11/16) Which other technologies would you like to use? Social login (Google,
Facebook, Twitter)
? (12/16) Which Framework would you like to use for the client? [BETA] React
? (13/16) Would you like to use the LibSass stylesheet preprocessor for your CS
S? Yes
? (14/16) Would you like to enable internationalization support? Yes
? Please choose the native language of the application English
? Please choose additional languages to install French
? (15/16) Besides JUnit and Karma, which testing frameworks would you like to u
se?
? (16/16) Would you like to install other generators from the JHipster Marketpl
ace? No

Installing languages: en, fr
events.js:160
throw er; // Unhandled 'error' event
^

TypeError: _this[method] is not a function
at blockTemplate.templates.forEach (C:\Data\workspace_perso\github-forks\gen
erator-jhipster\generators\generator-base.js:1933:42)
at Array.forEach (native)
at constructor.writeFilesToDisk (C:\Data\workspace_perso\github-forks\genera
tor-jhipster\generators\generator-base.js:1910:45)
at constructor.writeFiles (C:\Data\workspace_perso\github-forks\generator-jh
ipster\generators\client\files-react.js:409:10)
at constructor.writing (C:\Data\workspace_perso\github-forks\generator-jhips
ter\generators\client\index.js:366:36)
at Object. (C:\Data\workspace_perso\github-forks\generator-jhipst
er\node_modules\yeoman-generator\lib\index.js:417:23)
at C:\Data\workspace_perso\github-forks\generator-jhipster\node_modules\run-
async\index.js:25:25
at C:\Data\workspace_perso\github-forks\generator-jhipster\node_modules\run-
async\index.js:24:19
at C:\Data\workspace_perso\github-forks\generator-jhipster\node_modules\yeom
an-generator\lib\index.js:418:9
at runCallback (timers.js:672:20)

C:\Data\workspace_perso\github-forks\jhipster-react-branch>

Contributor

gmarziou commented Jul 19, 2017 edited

I got same error when I tried this morning with 2 languages too.
Maybe try without i18n first.

Member

deepu105 commented Jul 19, 2017

Yes i18n might not work, anyways I'm planning to rewrite to typescript and use bootstrap over the weekend, so next week I hope it can make some basic apps

Member

andidev commented Jul 19, 2017 edited

I would suggest using redux saga with react redux instead of the Promise Middleware. Seems like a more cleaner approach. I'm no expert on react though.

Member

deepu105 commented Jul 20, 2017

I have looked at Saga and its an overkill for crud operations. Its useful if you have really complex side effects to manage. Also redux advices to start simple with promise and thunk and use saga or observable only when you have complex scenarios. Also you can always use multiple libs with redux on same app as all are middlewares. So you could use promise + thunk for simple use cases and when you have complex ones you could use saga or observable

Contributor

sendilkumarn commented Jul 20, 2017

@deepu105 let me know once you have committed the code 👍 This is gonna be react-ful week 😝

Contributor

sdoxsee commented Jul 21, 2017 edited

I'm brand new to react but have been playing with it this week and discovered create-react-app that, as I'm sure most of you are already familiar with, hides details (webpack, babel, etc) in a react-scripts dependency that is managed and updated. Feels like a spring boot dependency bom. I suppose with typescript we'd already have to "eject" but I do quite like delegating the auto updating of react app dependencies to the maintainers of this >30000 star project. If typescript isn't set in stone, it would be awesome if we could leverage the tool without ejecting. Do you think that would be possible or desirable? Again...I'm new...which is why I don't want to configure with tools that keep shifting....grunt, gulp, webpack... :) The convention sounds really attractive.

Refs:
- https://medium.com/@tuchk4/why-i-love-create-react-app-e63b1be689a3
- https://github.com/facebookincubator/create-react-app
- https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/template/README.md

Update:
As for typescript, there's this fork https://github.com/wmonk/create-react-app-typescript that also seems to have the backing of Microsoft? https://github.com/Microsoft/TypeScript-React-Starter

Member

deepu105 commented Jul 21, 2017

Member

deepu105 commented Jul 23, 2017

@sendilkumarn I have committed initial version of TS + BS code. You need to enable sass to test it. I think there are some quirks with i18n and the header menu needs fixing, but feel free to hack on

Member

deepu105 commented Jul 23, 2017

my test config

{
  "generator-jhipster": {
    "promptValues": {
      "packageName": "com.mycompany.myapp",
      "nativeLanguage": "en"
    },
    "jhipsterVersion": "4.6.2",
    "baseName": "jhReactTest",
    "packageName": "com.mycompany.myapp",
    "packageFolder": "com/mycompany/myapp",
    "serverPort": "8080",
    "authenticationType": "jwt",
    "hibernateCache": "ehcache",
    "clusteredHttpSession": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "h2Disk",
    "prodDatabaseType": "mysql",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSocialSignIn": false,
    "jwtSecretKey": "788e69b63dbdc967046927b8e5855fda73e9027c",
    "clientFramework": "react",
    "useSass": false,
    "clientPackageManager": "yarn",
    "applicationType": "monolith",
    "testFrameworks": [],
    "jhiPrefix": "jhi",
    "enableTranslation": true,
    "nativeLanguage": "en",
    "languages": [
      "en"
    ]
  }
}
Member

deepu105 commented Jul 23, 2017

@sendilkumarn also its mostly not templated yet so other options needs to be templated in and i18n needs to be handled. I'll come up with a solution for i18n

Member

pascalgrimaud commented Jul 23, 2017

@deepu105 : soon holidays for me but, I will try to add:

survivant commented Jul 24, 2017 edited

I tried this morning, but I got this at the end

ERROR in ./src/main/webapp/app/index.tsx
Module not found: Error: Can't resolve './config/translation' in 'C:\Data\worksp
ace_perso\github-forks\test-branch-jhreact\src\main\webapp\app'
 @ ./src/main/webapp/app/index.tsx 13:20-51
 @ multi react-hot-loader/patch ./src/main/webapp/app/index

ERROR in ./src/main/webapp/app/app.tsx
Module not found: Error: Can't resolve './reducers/locale' in 'C:\Data\workspace
_perso\github-forks\test-branch-jhreact\src\main\webapp\app'
Contributor

sendilkumarn commented Jul 26, 2017 edited

@deepu105 I added few fixes there. Waiting for your i18n 😉

@survivant the errors that you have mentioned should disappear now.

Member

deepu105 commented Jul 26, 2017

@sendilkumarn great thanks. Feel free to take up the next task 😄

Member

deepu105 commented Jul 26, 2017

I have sample unit tests, will commit those when I find time

Member

deepu105 commented Aug 11, 2017

Base configuration works now
image

Member

deepu105 commented Aug 11, 2017

Karma tests works as well

Member

deepu105 commented Aug 11, 2017

@addisoc is working on the settings and password page

Contributor

gzsombor commented Aug 11, 2017

You work like a robot ! 👍

hey, anyone could tell me how can i create an application using react ?

Contributor

gmarziou commented Aug 20, 2017 edited

@filiipake this is written in this issue: #6044 (comment)

Be warned that this is work in progress.

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