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

React js integration proof of concept #30

Closed
stefanrinderle opened this issue Jul 17, 2016 · 6 comments
Closed

React js integration proof of concept #30

stefanrinderle opened this issue Jul 17, 2016 · 6 comments
Assignees

Comments

@stefanrinderle
Copy link
Owner

As stated in #20 we want to refactor the frontend code to be able to extend the plugin functionality fast and reliable. The POC will be implemented in branch softvis3d_react_example and then hopefully merged into master.

Acceptance criteria:

  • Integrate a simple react component somewhere in the angular app view
  • Write a plain typescript class which can be used within the current angular app view
  • Write simple tests with karma/jasmine for the typescript class
@stefanrinderle stefanrinderle added this to the Frontend code refactoring milestone Jul 17, 2016
@stefanrinderle stefanrinderle self-assigned this Jul 17, 2016
stefanrinderle added a commit that referenced this issue Jul 17, 2016
stefanrinderle added a commit that referenced this issue Jul 17, 2016
@stefanrinderle
Copy link
Owner Author

All of the acceptance criteria is done:

  • Integrate a simple react component somewhere in the angular app view
  • Write a plain typescript class which can be used within the current angular app view
  • Write simple tests with karma/jasmine for the typescript class

What i still want to check is how the two applications can talk to each other. Example implementation can be found here:
https://www.bimeanalytics.com/engineering-blog/you-put-your-react-into-my-angular/

This would enable us to replace one component after another which would be great.

Open issues:
Build is not optimal yet. Javascript console shows:
"WARNING: Tried to load angular more than once."

@stefanrinderle
Copy link
Owner Author

Although it was pretty easy to integrate react within the same webpage as the existing angularjs app, it is pretty hard to integrate the two things. Especially if it gets to communication with each other and haven an react component within an angular app. As far as i have seen for now, I think this would make everything more complicated instead of easy - which was the initial though for this.

So, i would suggest the following:

Current version 0.6.0:

  • Start to rewrite most of the business logic code from JS to Typescript. This integration is easy and works well.
    • LayoutProcessor
    • Softvis3dModel
    • Parts of FileLoaderController
    • TreeService
    • Parts ViewerService
  • Remove the react code for now

Next version 0.6.1:

  • Completely switch from angularjs to react in one big bang

From my point of view, although it is a big bang then, it gets us focussed not on the framework but on the features. What do you think?

@yvo-niedrich
Copy link
Collaborator

Sounds like a solid strategy.

Do we want to refactor the business logic as well for version 0.6? I would love to separate the "drawing configuration" a little bit more from the "infobar and user interaction". But the old behavior still has to work with a separate abstraction layer, so it might be a lot more work...

Also (for the build chain) i would totally recommend the typescript build to fail, if the linter or tests don't succeed. At the Moment the Java-Build still exits with code 0 should webpack fail, but the result contains outdated or no javascript code. Also there are theese 1k linter warnings 😉

This is my tslint configuration with a strict build chain for CCV

{
    "extends": "tslint:latest",
    "rules": {
        "max-line-length": [true, 125],
        "ordered-imports": false,
        "object-literal-sort-keys": false,
        "interface-name": [true, "never-prefix"],
        "variable-name": ["check-format", "allow-leading-underscore"],
        "trailing-comma": [true, "never"],
        "no-bitwise": false,
        "quotemark": [true, "double"],
        "eofline": false,
        "no-constructor-vars": true
    }
}```

@stefanrinderle
Copy link
Owner Author

As we have to rewrite the whole UI code within 0.6 - i think that this could be any easy point to change a lot in the UI logic as well. So I would do #34 within the refactoring from agnularjs to react. Otherwise we would rewrite the code twice.

About the java build. The build fails if the Typescript code does not compile as well on test failures. For the javascript code, this was true. I added the --bail options to the webpack build. This causes also javascript errors to fail the build.

As we want to completely switch to typescript, i would ignore the javascript errors but create a solid build chain for the typescript code. I copied your tslint.conf and added tslint to the build to fail on errors and on warnings.

@yvo-niedrich
Copy link
Collaborator

yvo-niedrich commented Aug 22, 2016

I'd recommend using TypeScript 2.0 (beta) asap. The changes are quite significant and a later migration would be even more painful, once the alle the modules are already written in TS 1.8.

For updating change the version for typescript in package.json to "^2.0.0" or "beta" will suffice.

Also for one of the best changes in TS2.0 the option strictNullChecks needs to be enabled in the tsconfig.json. For more Info on strictNullChecks see here


What is new in TypeScript 2.0

stefanrinderle added a commit that referenced this issue Aug 29, 2016
stefanrinderle added a commit that referenced this issue Aug 31, 2016
stefanrinderle added a commit that referenced this issue Sep 3, 2016
stefanrinderle added a commit that referenced this issue Sep 3, 2016
stefanrinderle added a commit that referenced this issue Sep 3, 2016
stefanrinderle added a commit that referenced this issue Sep 3, 2016
stefanrinderle added a commit that referenced this issue Sep 3, 2016
stefanrinderle added a commit that referenced this issue Sep 4, 2016
stefanrinderle added a commit that referenced this issue Sep 4, 2016
@stefanrinderle
Copy link
Owner Author

All classes but the following are now written in typescript:

  • UI controller
  • AngularJS framework bootstrap classes
  • base-frontend.model folder (no typescript definitions available yet)

I would propose to close this issue and do the rest of the work in the next milestone "Frontend code refactoring 2".

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

2 participants