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

Function traverseSnapshot not properly assembling the new translated route. #151

Open
JoseCMRocha opened this issue Dec 7, 2018 · 11 comments

Comments

@JoseCMRocha
Copy link

JoseCMRocha commented Dec 7, 2018

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
Copy link

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

@JoseCMRocha
Copy link
Author

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
Copy link

@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
Copy link

dragorwyin commented Feb 26, 2019

@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
Copy link
Author

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
Copy link

dragorwyin commented Feb 26, 2019

@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.

@MarcARoberge
Copy link

Hi, just wondering when @CyWolf is going to be merged to fix this issue?

@hohler
Copy link

hohler commented Mar 19, 2019

@MarcARoberge as @CyWolf does not have time for tests, I just created a PR from his fork. We need this fix.

@giacomo
Copy link

giacomo commented Apr 18, 2019

any news on this?

@relvingonzalez
Copy link

Are there any plans for this?

@giacomo
Copy link

giacomo commented Jun 12, 2019

for a patch see #159 (comment)

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

7 participants