chore: Upgrade to Angular v14 #186

rgant · 2023-04-12T19:39:13Z

DESCRIPTION:
Angular v13 support window ends 2023-05-04. Angular v16 is expected 2023-05-01 (BTW documentation does not express support for Angular v15 currently. But the code does. Should this be updated?)
My analysis of the current Angular code shows nothing that is a breaking change from Angular v13 to v15.

Additionally I have refreshed the dependencies and updated the lint configuration to current recommendations from Angular / @angular-eslint/eslint-plugin. All code changes are automatic formatting and fixing from tooling.

Finally I have run npx package-json-validator --warnings --recommendations and npx sort-package-json.

STEPS TO TEST:

ng lint
ng test
ng build

All linting and tests pass on my system. Build appears to be good as well. All of my future contributions to this library will likely be based off of these changes.

__DESCRIPTION:__ Angular v13 support window ends [2023-05-04](https://angular.io/guide/releases#actively-supported-versions). Angular v16 is expected [2023-05-01](https://angular.io/guide/releases#release-schedule) (BTW [documentation](https://developers.transifex.com/docs/angular-sdk) does not express support for Angular v15 currently. But the [code](https://github.com/transifex/transifex-javascript/blob/master/packages/angular/projects/tx-native-angular-sdk/package.json#L19-L21) does. Should this be updated?) My analysis of the current Angular code shows nothing that is a breaking change from Angular v13 to v15. Additionally I have refreshed the dependencies and updated the lint configuration to current recommendations from Angular / @angular-eslint/eslint-plugin. All code changes are automatic formatting and fixing from tooling. Finally I have run `npx package-json-validator --warnings --recommendations` and `npx sort-package-json`. __STEPS TO TEST:__ 1. `ng lint` 2. `ng test` 3. `ng build` All linting and tests pass on my system. Build appears to be good as well. All of my future contributions to this library will likely be based off of these changes.

* chore: Upgrade to Angular v14 __DESCRIPTION:__ Angular v13 support window ends [2023-05-04](https://angular.io/guide/releases#actively-supported-versions). Angular v16 is expected [2023-05-01](https://angular.io/guide/releases#release-schedule) (BTW [documentation](https://developers.transifex.com/docs/angular-sdk) does not express support for Angular v15 currently. But the [code](https://github.com/transifex/transifex-javascript/blob/master/packages/angular/projects/tx-native-angular-sdk/package.json#L19-L21) does. Should this be updated?) My analysis of the current Angular code shows nothing that is a breaking change from Angular v13 to v15. Additionally I have refreshed the dependencies and updated the lint configuration to current recommendations from Angular / @angular-eslint/eslint-plugin. All code changes are automatic formatting and fixing from tooling. Finally I have run `npx package-json-validator --warnings --recommendations` and `npx sort-package-json`. __STEPS TO TEST:__ 1. `ng lint` 2. `ng test` 3. `ng build` All linting and tests pass on my system. Build appears to be good as well. All of my future contributions to this library will likely be based off of these changes. * trim white space

rgant · 2023-04-15T12:21:19Z

Merging this branch will close #187

pablotransifex · 2023-04-15T15:51:52Z

@rgant I appreciate the effort you put in linting and cleaning the package. We in the beginning decided to go with the Airbnb linting standards in general for the TXNative repository, but I actually was aware of the specific linting rules and standards in Angular development. I'll check these changes and let you know how we decided to proceed.

Again, thank you very much.

pablotransifex · 2023-04-15T16:31:07Z

@rgant It seems that the tests have some problem, in order to reproduce locally, please run in the root folder of the repo:

npm run bootstrap

and in the angular folder:

npm test

We are using Lerna to build.

rgant · 2023-04-16T14:05:39Z

I'll look into the error, but best I can tell that is a build error and has less to do with my code and more to do with old versions of lerna and old versions of Angular (14 is still old) with brand new versions of NodeJS v18.

I tend to expect that Transifex isn't going to wholly accept my changes. Which is fine, but I did promise to suggest improvements before based on my company's experience using your tools. We have completely abandoned this Angular implementation because of problems with how it works. I will be completely re-writing everything here to work in (what I believe is) a more Angular-y way.

It's up to you if you want to wait to see the final results before reviewing, or review individually. I won't open any other pull requests here, but I will be recording them in my fork for you to look at if you desire.

pablotransifex · 2023-04-18T07:19:26Z

@rgant It seems so, a build problem, indeed, but I don't know how to solve it yet. I'll look into it too.

I'll be checking your changes in your forked repository a well, thank you!

rgant · 2023-04-18T17:48:27Z

I believe the issue is that the library's package.json includes Angular 12 and 13. But I haven't had a chance yet to try and remove those.

pablotransifex · 2023-04-19T08:55:47Z

@rgant The problem was this part in the package.json inside the library:

  "dependencies": {
    "tslib": "^2.3.0"
  }

this generates a node_modules inside it that is not the same as in the root folder, and thus the discrepancy.

I pushed a commit, let's see if now it's ok (locally the tests ran ok).

rgant · 2023-04-19T15:05:27Z

Good catch, I will add documentation about removing that since that is the default when Angular v14 creates a library.

pablotransifex

@rgant Looks good, but I want to test with older versions of Angular. Did you test in a project with v12/13 of Angular?

pablotransifex · 2023-04-21T09:04:59Z

packages/angular/projects/tx-native-angular-sdk/tests/T.decorator.spec.ts

+    const component = fixture.componentInstance;
+
+    fixture.detectChanges();
+    expect(tx.translate).toHaveBeenCalledTimes(2); // Why twice?


I think we can ignore this, and use a simple hasBeenCalled() . I couldn't find any reason why it's called twice... probably some internal mechanism for decorators? Let's assume that is working fine.

pablotransifex · 2023-04-21T09:05:05Z

packages/angular/projects/tx-native-angular-sdk/tests/T.decorator.spec.ts

+    const component = fixtureWithInstance.componentInstance;
+
+    fixtureWithInstance.detectChanges();
+    expect(tx.translate).toHaveBeenCalledTimes(2); // Why twice?


I think we can ignore this, and use a simple hasBeenCalled() . I couldn't find any reason why it's called twice... probably some internal mechanism for decorators? Let's assume that is working fine.

pablotransifex · 2023-04-21T09:09:43Z

packages/angular/projects/tx-native-angular-sdk/src/lib/language-picker.component.ts

    // when the instance is ready using a subscription
-    if (this.instance && this.instance.alias) {
+    if (this.instance.alias) {


Maybe use this.instance?.alias just to be sure?

pablotransifex · 2023-05-09T09:30:59Z

@rgant I have errors when compiling in Angular 12 and 13, like this one:

using this PR is breaking those versions, is this expected?

rgant · 2023-05-09T10:59:17Z

Angular 12 and 13 are no longer supported versions of Angular: https://angular.io/guide/releases#actively-supported-versions

pablotransifex · 2023-05-09T11:47:54Z

@rgant correct, I was thinking in creating a new release with the v14+ for angular and keep another release for upto v13. This way we can support older versions with the current implementation, and improve the new installations with your refactoring (when ready).

From my side there is no problem, I would like to discuss this and back to you.

pablotransifex · 2023-05-10T08:59:05Z

@rgant correct, I was thinking in creating a new release with the v14+ for angular and keep another release for upto v13. This way we can support older versions with the current implementation, and improve the new installations with your refactoring (when ready).

What do you think?

rgant · 2023-05-10T14:11:01Z

That is one way. I think a more typical way is to say that users of old versions of frameworks should install old versions of add-ons.

egilkh · 2023-08-18T05:44:42Z

I agree with rgrant. And hope this can be merged :)

pablotransifex · 2023-08-18T07:23:06Z

I agree with rgrant. And hope this can be merged :)

@egilkh Due to vacations we cannot do it now, but we will as soon as possible.

pablotransifex · 2023-08-22T11:31:18Z

@rgant Can you please rebase and resolve conflict in this PR in order to move on and release?

pablotransifex

@rgant I don't know what changed since last time, but there are errors in the compilation, related to angular dependencies, also notices that you added node and npm versions to package.json, that should not be there.

Last time we build this PR was good, I don't know what changed and now is breaking.

pablotransifex · 2023-08-23T08:57:02Z

packages/angular/package.json

-    "typescript": "~4.4.4"
+    "typescript": "~4.7.4"
+  },
+  "engines": {


@rgant We need to remove this, I think.

pablotransifex · 2023-08-23T08:57:24Z

@rgant I don't know what changed since last time, but there are errors in the compilation, related to angular dependencies, also notices that you added node and npm versions to package.json, that should not be there.

Last time we build this PR was good, I don't know what changed and now is breaking.

pablotransifex · 2023-09-27T11:56:05Z

@rgant Closing this, we have fixed the build on another PR already merged. Feel free to let us know any other improvements that we can apply for the Angular 14 version we will release soon. And thank you very much again for the effort.

rgant added 7 commits April 12, 2023 15:30

trim white space

bc2a88d

Sort eslint extends

a2e711c

Merge branch 'chore/update-angular-v14'

95caa05

Use optional chaining

c167870

Merge branch 'chore/update-angular-v14'

aabb8eb

rgant mentioned this pull request Apr 15, 2023

[Bug] Angular @T Decorator #187

Open

Fixt @t Decorator

b2a8dc4

Merge branch 'chore/update-angular-v14'

b102199

pablotransifex reviewed Apr 21, 2023

View reviewed changes

rgant and others added 2 commits August 22, 2023 11:09

Merge remote-tracking branch 'upstream/master'

3c76933

Fix test execution

526e432

rgant force-pushed the chore/update-angular-v14 branch from 6f4a5d3 to 526e432 Compare August 22, 2023 15:11

pablotransifex reviewed Aug 23, 2023

View reviewed changes

tse018 mentioned this pull request Sep 22, 2023

feat(chore): Upgrade Angular v14 #191

Merged

pablotransifex closed this Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Upgrade to Angular v14 #186

chore: Upgrade to Angular v14 #186

rgant commented Apr 12, 2023

rgant commented Apr 15, 2023

pablotransifex commented Apr 15, 2023 •

edited

Loading

pablotransifex commented Apr 15, 2023 •

edited

Loading

rgant commented Apr 16, 2023 •

edited

Loading

pablotransifex commented Apr 18, 2023

rgant commented Apr 18, 2023

pablotransifex commented Apr 19, 2023 •

edited

Loading

rgant commented Apr 19, 2023

pablotransifex left a comment

pablotransifex Apr 21, 2023

pablotransifex Apr 21, 2023

pablotransifex Apr 21, 2023

pablotransifex commented May 9, 2023

rgant commented May 9, 2023

pablotransifex commented May 9, 2023

pablotransifex commented May 10, 2023

rgant commented May 10, 2023

egilkh commented Aug 18, 2023

pablotransifex commented Aug 18, 2023

pablotransifex commented Aug 22, 2023

pablotransifex left a comment

pablotransifex Aug 23, 2023

pablotransifex commented Aug 23, 2023

pablotransifex commented Sep 27, 2023

chore: Upgrade to Angular v14 #186

chore: Upgrade to Angular v14 #186

Conversation

rgant commented Apr 12, 2023

rgant commented Apr 15, 2023

pablotransifex commented Apr 15, 2023 • edited Loading

pablotransifex commented Apr 15, 2023 • edited Loading

rgant commented Apr 16, 2023 • edited Loading

pablotransifex commented Apr 18, 2023

rgant commented Apr 18, 2023

pablotransifex commented Apr 19, 2023 • edited Loading

rgant commented Apr 19, 2023

pablotransifex left a comment

Choose a reason for hiding this comment

pablotransifex Apr 21, 2023

Choose a reason for hiding this comment

pablotransifex Apr 21, 2023

Choose a reason for hiding this comment

pablotransifex Apr 21, 2023

Choose a reason for hiding this comment

pablotransifex commented May 9, 2023

rgant commented May 9, 2023

pablotransifex commented May 9, 2023

pablotransifex commented May 10, 2023

rgant commented May 10, 2023

egilkh commented Aug 18, 2023

pablotransifex commented Aug 18, 2023

pablotransifex commented Aug 22, 2023

pablotransifex left a comment

Choose a reason for hiding this comment

pablotransifex Aug 23, 2023

Choose a reason for hiding this comment

pablotransifex commented Aug 23, 2023

pablotransifex commented Sep 27, 2023

pablotransifex commented Apr 15, 2023 •

edited

Loading

pablotransifex commented Apr 15, 2023 •

edited

Loading

rgant commented Apr 16, 2023 •

edited

Loading

pablotransifex commented Apr 19, 2023 •

edited

Loading