Function traverseSnapshot not properly assembling the new translated route.

JoseCMRocha commented 5 years ago

I'm submitting a ... (check one with "x")

[ x] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if similar feature request does not exist
[ ] support request => Suggested place for help and support is [stackoverflow](https://stackoverflow.com/), search for similar question before posting

Description

Using angular 7+ if a route as parameters, for example :name or :id, this method on this line "...Object.keys(snapshot.params).length ? [snapshot.params] : []," will add to the route the following ";name=valueofthename" or ";id=valueoftheid". There is another problem on the line above on the urlPart this urlPart I think needs to be splited by "/" otherwise if my route is something like "person/:name" the the result of calling the method changeLanguage that calls the traverseSnapshot will result in something like "lang/person%2Fvalueofthename;name=valueofthename".

🔬 Minimal Reproduction (if applicable)

On this code https://stackblitz.com/edit/localize-router-v2-issue-template-o6mvus if you write on the link "/en/hello/name" and after click on on of the buttons to change the language you will sew on the console the error "Cannot match any routes. URL Segment: 'de/hello%2Fname;name=name'" the type of the error does not matter for the problem as it is just a not matching error, the problem is actully the construction of the url that is not properly constructed.

🌍 Your Environment

Angular Version:


Angular CLI: 7.0.5
Node: 10.13.0
OS: win32 x64
Angular: 7.0.3
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.10.5
@angular-devkit/build-angular     0.10.5
@angular-devkit/build-optimizer   0.10.5
@angular-devkit/build-webpack     0.10.5
@angular-devkit/core              7.0.5
@angular-devkit/schematics        7.0.5
@angular/cli                      7.0.5
@ngtools/webpack                  7.0.5
@schematics/angular               7.0.5
@schematics/update                0.10.5
rxjs                              6.3.3
typescript                        3.1.6
webpack                           4.19.1

Localize Router Version:


localize-router: 2.0.0-RC.2 
localize-router-http-loader: 1.0.2

Anything else relevant?


@ngx-translate/core: 11.0.1
@ngx-translate/http-loader: 4.0.0

andrewwhitehead commented 5 years ago

I have a potential fix for this here: https://github.com/cywolf/localize-router/tree/fix-151

JoseCMRocha commented 5 years ago

Hi @cywolf just tested that changes and it seems to fix the problem that I described. Did you reported this problem previously on this repository? Asking to see if there is more discussion about the problem reported. Did you try to create a pull request of your change to this greentube/localize-router? I verified that your solution fix my problem but I don't know if it will break something else.

andrewwhitehead commented 5 years ago

@JoseCMRocha Hiya, I haven't reported the issue separately, and haven't created a PR as I don't have time to add unit test(s) as the moment.

dragorwyin commented 5 years ago

@cywolf Your solution not working for me when I have translated first slug in path, for example: /en/register/token and /pl/rejestracja/token. After language change it moves me into /pl/rejestracja without handling :token parameter. Even if I did fork and just copied your file. It's huge bug for me actually :?

JoseCMRocha commented 5 years ago

Hi @dragorwyin that is strange your case is very similar with mine and the solution by @cywolf worked for me, did you check your routing configuration to see if there is something wrong? are you able to create a example to reproduce your problem?

dragorwyin commented 5 years ago

@JoseCMRocha Here is a reproduction: I have a app-routing.module.ts file with simple configuration:

import { Routes } from '@angular/router';
import { FullComponent } from './core/layouts/full/full.component';
import { BlankComponent } from './core/layouts/blank/blank.component';
import { NotFoundComponent } from './modules/not-found/not-found.component';
import { LoginComponent } from './modules/login/login.component';
import { AuthGuardConfirmed } from './core/guard/auth-guard-confirmed.service';

export const routes: Routes = [
  {
    path: '',
    component: FullComponent,
    canLoad: [AuthGuardConfirmed],
  }, {
    path: '',
    component: BlankComponent,
    children: [{
      path: 'login',
      component: LoginComponent
    }, {
      path: 'register',
      loadChildren: './modules/register/register.module#RegisterModule',
    }]
  },
  { path: '404', component: NotFoundComponent },
  { path: '**', redirectTo: '/404' }
];

app.module.ts

import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';

import { AppComponent } from './app.component';
import { FullComponent } from './core/layouts/full/full.component';
import { BlankComponent } from './core/layouts/blank/blank.component';
import { NotFoundComponent } from './modules/not-found/not-found.component';
import { LoginComponent } from './modules/login/login.component';
import { AuthGuardConfirmed } from './core/guard/auth-guard-confirmed.service';
import { TranslateHttpLoader } from '@ngx-translate/http-loader';

import { HttpClientModule, HttpClient } from '@angular/common/http';

import { TranslateModule, TranslateService, TranslateLoader } from '@ngx-translate/core';
import { LocalizeRouterModule, LocalizeRouterSettings, LocalizeParser } from 'localize-router';
import { LocalizeRouterHttpLoader } from 'localize-router-http-loader';
import { routes } from './app-routing.module';
import { RouterModule } from '@angular/router';
import { Location } from '@angular/common';

export function createTranslateLoader(http: HttpClient) {
  return new TranslateHttpLoader(http, './assets/i18n/', '.json');
}

export function HttpLoaderFactory(http: HttpClient) {
  return new TranslateHttpLoader(http);
}

@NgModule({
  declarations: [
    AppComponent,
    BlankComponent,
    NotFoundComponent,
    FullComponent,
    LoginComponent,
  ],
  imports: [
    BrowserModule,
    BrowserAnimationsModule,
    HttpClientModule,
    TranslateModule.forRoot({
      loader: {
        provide: TranslateLoader,
        useFactory: HttpLoaderFactory,
        deps: [HttpClient]
      }
    }),
    LocalizeRouterModule.forRoot(routes, {
      parser: {
        provide: LocalizeParser,
        useFactory: (translate, location, settings, http) =>
          new LocalizeRouterHttpLoader(translate, location, settings, http),
        deps: [TranslateService, Location, LocalizeRouterSettings, HttpClient]
      }
    }),
    RouterModule.forRoot(routes)
  ],
  providers: [
    AuthGuardConfirmed,
  ],
  bootstrap: [AppComponent]
})
export class AppModule { }

register.routing.ts:

import { Routes, RouterModule } from '@angular/router';
import { RegisterComponent } from './register.component';
import { ConfirmComponent } from './confirm/confirm.component';

export const routes: Routes = [
  {
    path: '',
    children: [{
      path: '',
      component: RegisterComponent
    }, {
      path: ':token',
      component: ConfirmComponent
    }]
  }
];

export const RegisterRoutes = RouterModule.forChild(routes);

register.module.ts

import { NgModule } from '@angular/core';
import { CommonModule } from '@angular/common';
import { routes } from './register.routing';

import { MaterialModule } from '../../shared/material.module';
import { SharedModule } from '../../shared/shared.module';

import { ComponentsModule } from '../../components/components.module';
import { RegisterComponent } from './register.component';
import { ConfirmComponent } from './confirm/confirm.component';
import { TranslateModule, TranslateLoader } from '@ngx-translate/core';
import { LocalizeRouterModule } from 'localize-router';
import { RouterModule } from '@angular/router';

@NgModule({
  imports: [
    CommonModule,
    SharedModule,
    MaterialModule,
    ComponentsModule,
    TranslateModule,
    LocalizeRouterModule.forChild(routes),
    RouterModule.forChild(routes)
  ],
  declarations: [
    RegisterComponent,
    ConfirmComponent,
  ],
})
export class RegisterModule { }

I have translations in assets/i18n (pl.json and en.json):

pl.json: "{ ROUTES.register": "rejestracja" }",
en.json: "{ ROUTES.register": "register" }",

I have an locales.json in assets:

{
  "locales": [
    "en",
    "pl"
  ],
  "prefix": "ROUTES."
}

I hope there will be any solution and will be implemented soon in this library ;P

There is also another problem: I cannot translate child name, for example: /pl/rejestracja/potwierdz is an translation of /en/register/confirm. I tried translate confirm slug without any success. Tried use

{ ROUTES.confirm: "potwierdz" },
{ ROUTES.register.confirm: "potwierdz" } and
{ ROUTES.register.confirm: "rejestracja/potwierdz" } without success.