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

Angular 7 compatibility #36

Closed
sabeurch opened this issue Jun 7, 2018 · 20 comments
Closed

Angular 7 compatibility #36

sabeurch opened this issue Jun 7, 2018 · 20 comments

Comments

@sabeurch
Copy link
Contributor

sabeurch commented Jun 7, 2018

Actually the Angular Slick grid is not compatible Angular 6, errors of kind :

node_modules/angular-slickgrid/app/modules/angular-slickgrid/models/backendEventChanged.interface.d.ts(3,10): error TS2305: Module '"E:/WS/Angular6App/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/angular-slickgrid/app/modules/angular-slickgrid/models/backendServiceApi.interface.d.ts(5,10): error TS2305: Module '"E:/WS/Angular6App/node_modules/rxjs/Observable"' has no exported member 'Observable'.
node_modules/angular-slickgrid/app/modules/angular-slickgrid/services/export.service.d.ts(3,10): error TS2305: Module '"E:/WS/Angular6App/node_modules/rxjs/Subject"' has no exported member 'Subject'.

To by pass this issue, I downgraded the RXJS version from 6.0.0 to 5.6.0-forward-compat.5
This is a temporary solution, all errors related to angular slick grid disapeared, but when starting the application, this alert is displayed (in red):

This project uses a temporary compatibility version of RxJs (5.6.0-forward-compat.5).
Please visit the link below to find instructions on how to update RxJs.
https://docs.google.com/document/d/12nlLt71VLKb-z3YaSGzUfx6mJbc34nsMXtByPUN35cg/edit# 

Should this be worrying ?

@ghiscoding
Copy link
Owner

The RxJS dependency is in devDependency, technically you should be able to install any version. I didn't try Angular 6 yet, I'm still too busy trying to release the new 1.x Major version.

If I look at one of your error with Export Service on this [line] it's imported this way

import { Subject } from 'rxjs/Subject';

did that change in version 6? I need to support both RxJS 5 and 6

@ghiscoding
Copy link
Owner

ghiscoding commented Jun 7, 2018

I see they changed their imports yet again (using this article as reference)

Observable, Subject etc.

import { Observable } from 'rxjs/Observable';`
import { Subject } from 'rxjs/Subject';

becomes

import { Observable, Subject } from 'rxjs';

I moved the RxJS 5 into the dependencies, not sure if that is a good thing though.
I really don't have time at this moment to work on Angular 6, if you have time to help me figure out a way to support both Angular/RxJS 5 & 6 that would be ideal.

I am also finished refactoring for the new Major Release, can you please reply me on this RxJS ASAP since I'm already doing a Major (breaking) release. Also make sure to follow the Migration Guide, the new version changed quite a lot to support multiple grids

@sabeurch
Copy link
Contributor Author

sabeurch commented Jun 8, 2018

Unfortunately, the support of C/C++ like macros is not yet supported in type script: microsoft/TypeScript#4892
Also, precompile hooks is not a good idea, so source code should be kept as it is.
Until now, I'll remain using the compatible rxjs version '5.6.0-forward-compat.5' as a temporary solution.
Do you have any planning on when to migrate into Angular 6 ? (I know this could only be done after the stablilty of current major version). Or maybe creating a separate branch for rxjs 6: This solution is not good as well, as we should perform the update twice.
For now, I am hopping angular team remain supporting the RXJS forward-compat version for a long time.

@sabeurch
Copy link
Contributor Author

sabeurch commented Jun 8, 2018

This is an offcial migration document https://github.com/ReactiveX/rxjs/blob/master/docs_app/content/guide/v6/migration.md . It accentuates on the necessity of refactoring source code after a step of compatibility.
To make Angular Slick grid temporarly working on Angular 6:
$npm install rxjs@6 rxjs-compat@6 --save
It will install "rxjs": "^6.2.0" as devDependencies and "rxjs-compat": "^6.2.0" as dependencies,

@ghiscoding
Copy link
Owner

So did you get it working without warnings? or are you saying that there's something to modify in my lib?

@sabeurch
Copy link
Contributor Author

sabeurch commented Jun 8, 2018

With the use of rxjs@6 and rxjs-compat@6 , the waring disapeared.
The angular slick grid works fine on Angular 6.
So, I have both rxjs v6 and rxjs-compat (for compatibility with angular 5) installed.
When all libraries (that I am using) finished migrating to Angular 6 , I 'll then drop the compatibility layer defined by rxjs-compat.
You will not have to change in Angular Slick grid for the moment, otherwise you will be supporting 2 versions of the source code (one compatible with Angular 4, 5 and other sources compatible with Angular 6)

@ghiscoding
Copy link
Owner

That's good news, I don't expect to support Angular 6 yet. Maybe in over a month but not now and I would not want to create 2 major versions just because of RxJS compatibility.

Thanks for this info, I will leave the issue open in case someone else has the problem

@sabeurch
Copy link
Contributor Author

sabeurch commented Jun 8, 2018

You Are Welcome 👍

@ctung
Copy link

ctung commented Jun 11, 2018

Here's an attempt at the basic grid in Angular6 with rxjs-compat

The grid looks exactly like what I got when I tried building it from angular.cli

Any ideas?

@ghiscoding
Copy link
Owner

@ctung
I'm not sure how to put that in Starblitz, I haven't used it much, but you are missing the CSS (or SASS) Styling, you should see the Wiki - Styling

@ctung
Copy link

ctung commented Jun 12, 2018

Thanks, that fixed it!
stackblitz

@sabeurch
Copy link
Contributor Author

@ctung
In your stackblitz, this error is displayed for 1 second (on startup)
image
Then I got the datagrid correctly displayed. Is there any thing wrong with your css import ?

@ghiscoding
Copy link
Owner

Just so everyone knows, since there is a known way to bypass this issue.
I will most probably wait for Angular 7 to be out before addressing this one.

There are a few reasons

  1. There is a know bypass mentioned in previous post, thanks to @sabeurch
  2. I would like to finish working on all features
    • last major feature to work on is Editor/Filter collection async which is in the work (should be out within a month or so)
  3. This will be a Major version 2.x since there are breaking changes because of newer RxJS, and I would rather wait to see if there's more breaking changes with Angular 7 to avoid making 2 Major versions back to back

So will all these reasons, don't expect any resolution any time soon.
Please use the bypass that was mentioned in previous post

@senthil1690
Copy link

When we expect angular 6 support? We are in Angular-5 and decided to migrate Angular-6. Only bottle neck slickly grid for angular-6 migrate. So please provide the angular-6 supported version for us at earliest. We are planing to go live at the month of Aug'2018 end

@ghiscoding
Copy link
Owner

@senthil1690
I am in no rush to upgrade to RxJs 6 and because there is a known bypass described in previous post and because it's a breaking change so I have no intention to release a version 2.x of our lib. Just use rxjs-compat

Please remember this is an Open Source and free project, if you want to contribute that would be awesome, else just wait as I am only 1 developer mainly on this lib and I have much other bigger priorities.

@ghiscoding ghiscoding changed the title Angular 6 compatibility Angular 7 compatibility Dec 18, 2018
@ghiscoding
Copy link
Owner

ghiscoding commented Dec 18, 2018

@sabeurch @senthil1690 @ctung and everybody else...
I finally had time to update the lib to Angular 7 (skipped 6), I just pushed a Release Candidate version as 2.0.0 Beta.

Things worth to know

  • I'm skipping Angular 6 to only support Angular 7
  • ngx-translate had 2 major versions since Angular 5, but I will support only the latest version 11.0.0 which works with Angular 7
  • I updated only the Bootstrap 4 demo to Angular 7 (reasons below)
  • the new version 2.0.0 is a Major with Breaking Changes and has Beta tag until tomorrow.

You can install the RC version via (it's under a Beta tag for now, so you need to explicitly specify the version).
npm install [email protected] or yarn add [email protected]

For a sample of the upgrade changes in your App, you can take a look at the
Angular-Slickgrid Bootstrap 4 GitHub page repo

Note that I will keep the Bootstrap 3 demo on version 1.x.x of Angular-Slickgrid. I still wish to keep support of version 1 (which now has it's own branch) for the next few months. However major new features will only arrive in version 2.x.x of the lib.

FEEDBACK are welcome

I would like to have feedback if possible before tomorrow since I'm going on holiday starting tomorrow and I would like to make the version as official on NPM (it's under a Beta tag until tomorrow evening)

Some Wiki might need updates, I'll do that later. For now simply look at the Bootstrap 4 GitHub demo (you can look at the 2.0.0 update commit)

@ghiscoding
Copy link
Owner

Hey guys, I found a small problem with the date picker (Flatpickr) this morning, which I've now fixed. The problem was only showing in a production build and as I said it is now fixed.

I had no news but I decided to make it official, 2.0.1 is now the official version showing on NPM. I tried it on 2 App and I didn't see any other issues.

@sabeurch
Copy link
Contributor Author

@ghiscoding This is a good news , thank you so much for this good job 👌
I'll start testing your release ASAP.

@senthil1690
Copy link

@senthil1690
I am in no rush to upgrade to RxJs 6 and because there is a known bypass described in previous post and because it's a breaking change so I have no intention to release a version 2.x of our lib. Just use rxjs-compat

Please remember this is an Open Source and free project, if you want to contribute that would be awesome, else just wait as I am only 1 developer mainly on this lib and I have much other bigger priorities.

Thank you

@ghiscoding
Copy link
Owner

@senthil1690
A good way to say Thank You could be to up vote the library ⭐️
Thanks 😉

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

4 participants