Skip to content
This repository has been archived by the owner on Feb 25, 2022. It is now read-only.

Parameters are not decoded when retrieved. #120

Closed
kevindmoore opened this issue Oct 31, 2021 · 7 comments
Closed

Parameters are not decoded when retrieved. #120

kevindmoore opened this issue Oct 31, 2021 · 7 comments
Labels
bug Something isn't working fix awaiting confirmation

Comments

@kevindmoore
Copy link

kevindmoore commented Oct 31, 2021

If you pass strings as parameters that contain spaces, the spaces in the string gets encoded into %20 and not decoded on the way back.

Passing parameter:

context.pushNamed(detailsPageRouteName, params: {item: value});

Receiving:

      GoRoute(
        name: detailsPageRouteName,
        path: detailsPageRoute,
        pageBuilder: (context, state) => MaterialPage<void>(
          key: state.pageKey,
          child: Builder(
            builder: (context) {
              return Details(description: Uri.decodeComponent(state.params[item]!));
            },
          ),
        ),
      ),

I needed to add the Uri.decodeComponent to get it to work

@csells csells added the bug Something isn't working label Nov 3, 2021
csells added a commit that referenced this issue Nov 3, 2021
csells added a commit that referenced this issue Nov 3, 2021
@csells
Copy link
Owner

csells commented Nov 3, 2021

@kevindmoore I've got the test to repro the issue. working on a fix now.

@csells csells closed this as completed in 02c6820 Nov 7, 2021
@csells
Copy link
Owner

csells commented Nov 7, 2021

fixed in v2.2.2. @kevindmoore can you confirm?

@kevindmoore
Copy link
Author

I'm using v2.2.4 and I still see the issue

@csells
Copy link
Owner

csells commented Nov 7, 2021

can you post a minimal repro project? it's working in my tests.

@kevindmoore
Copy link
Author

kevindmoore commented Nov 8, 2021

Here is what I have on the 1 screen:

            onTap: () {
              final value = items[index];
              context.pushNamed(detailsPageRouteName, params: {item: value});
            },

where items is:

    final items = List<String>.generate(10000, (i) => 'Item $i');

Then my route is defined as:

     GoRoute(
        name: detailsPageRouteName,
        path: detailsPageRoute,
        pageBuilder: (context, state) => MaterialPage<void>(
          key: state.pageKey,
          child: Builder(
            builder: (context) {
              return Details(
                  description: Uri.decodeComponent(state.params[item]!));
            },
          ),
        ),

with constants:

const String detailsPageRouteName = '/details';
const String detailsPageRoute = '/details/:item';

Details is:

class Details extends StatelessWidget {
  final String description;

  const Details({Key? key, required this.description}) : super(key: key);
}

@csells
Copy link
Owner

csells commented Nov 10, 2021

@kevindmoore here's a minimal repro based on your snippets (I needed a fully running app to debug):

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

void main() => runApp(App());

class App extends StatelessWidget {
  App({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => MaterialApp.router(
        routeInformationParser: _router.routeInformationParser,
        routerDelegate: _router.routerDelegate,
        title: "GoRouter Example: Kevin's issue #120 repro",
      );

  late final _router = GoRouter(
    debugLogDiagnostics: true,
    routes: [
      GoRoute(
        path: '/',
        redirect: (_) => '/details',
      ),
      GoRoute(
        name: 'items',
        path: '/details',
        pageBuilder: (context, state) => MaterialPage<void>(
          key: state.pageKey,
          child: ItemsPage(),
        ),
      ),
      GoRoute(
        name: 'details',
        path: '/details/:item',
        pageBuilder: (context, state) => MaterialPage<void>(
          key: state.pageKey,
          child: DetailsPage(description: state.params['item']!), // ERROR!
        ),
      ),
    ],
    errorPageBuilder: (context, state) => MaterialPage<void>(
      key: state.pageKey,
      child: ErrorPage(state.error),
    ),
  );
}

class ItemsPage extends StatelessWidget {
  ItemsPage({Key? key}) : super(key: key);
  final items = List<String>.generate(10, (i) => 'Item $i');

  @override
  Widget build(BuildContext context) => Scaffold(
        body: ListView(children: [
          for (var index = 0; index != items.length; ++index)
            ListTile(
              title: Text(items[index]),
              onTap: () => context.pushNamed(
                'details',
                params: {'item': items[index]},
              ),
            )
        ]),
      );
}

class DetailsPage extends StatelessWidget {
  const DetailsPage({required this.description, Key? key}) : super(key: key);
  final String description;

  @override
  Widget build(BuildContext context) => Scaffold(body: Text(description));
}

class ErrorPage extends StatelessWidget {
  const ErrorPage(this.error, {Key? key}) : super(key: key);
  final Exception? error;

  @override
  Widget build(BuildContext context) => Scaffold(
        appBar: AppBar(title: const Text('Page Not Found')),
        body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: [
              Text(error?.toString() ?? 'page not found'),
              TextButton(
                onPressed: () => context.go('/'),
                child: const Text('Home'),
              ),
            ],
          ),
        ),
      );
}

A couple of notes:

  1. I wouldn't put slashes into your names; I've made the name for the DetailsPage route just details instead of /details. Names are just for looking up routes, not actually building them (that's what the path is for). It works either way but I find including slashes into the name is confusing.
  2. I wouldn't use push() or pushNamed() to build a stack; instead, I'd make the details route a sub-route of the "items" page (I had to guess at a name here, since you didn't provide one in you snippets) and use go() or goNamed() to build up the stack of pages. Check out the sub-routes docs for details.

I was able to duplicate your bug the repro above and then found the error in my tests that hid the bug (don't you hate that?!). I've fixed my tests, repro'd your bug and will be publishing a fix momentarily. Sorry for the delay. Keep those cards and letters coming!

@csells
Copy link
Owner

csells commented Nov 10, 2021

fixed in v2.2.5 (I think). @kevindmoore can you confirm?

@csells csells closed this as completed Nov 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working fix awaiting confirmation
Projects
None yet
Development

No branches or pull requests

2 participants