[go: nahoru, domu]

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

Fix example on 'Parse JSON in the background' page #6768

Closed
Whisper40 opened this issue May 16, 2021 · 16 comments
Closed

Fix example on 'Parse JSON in the background' page #6768

Whisper40 opened this issue May 16, 2021 · 16 comments
Labels
a.cookbook Relates to a cookbook recipe or guide cl.fixed Issue is closed as fixed e1-hours Effort: < 8 hrs lang.json Involves JSON formatted data p2-medium Necessary but not urgent concern. Resolve when possible.

Comments

@Whisper40
Copy link
Whisper40 commented May 16, 2021

Steps to Reproduce

Just follow what is writed there : https://docs.flutter.dev/cookbook/networking/background-parsing

Expected results:
The parsePhotos should not produce errors

Actual results:
Actually this method is all in red. I tried with and without this :

analyzer:
  strong-mode:
    implicit-casts: false
    implicit-dynamic: false

My code :

import 'dart:convert';
import 'package:flutter/foundation.dart';
import 'package:http/http.dart' as http;
import '../models/post_model.dart';

class HttpService {
  final String url = 'https://jsonplaceholder.typicode.com';
  final String postsURL = '/posts';
  final Map<String, String> headers = {
    'Content-Type': 'application/json'
  };



  List<Post> parsePosts(String responseBody) {
  final parsed = jsonDecode(responseBody).cast<Map<String, dynamic>>();

  return parsed.map<Post>((json) => Post.fromJson(json)).toList();
  }


  Future<List<Post>> fetchPosts() async {

    final http.Response response = await http.get(Uri.https(url, postsURL));

    if (response.statusCode == 200) {
      return compute(parsePosts,response.body);
    } else {
      throw Exception("Failed to load posts ${response.statusCode}");
    }
  }


}

image
image

@iapicca
Copy link
iapicca commented May 16, 2021

@Whisper40
I think the underlying issue is that too often codelabs and documentation in general
ignores effective dart and mislead developers

for the case you are reporting the code ignores avoid-using-cast

imho the issue should be codelabs and code samples should enforce effective dart guidelines

@Whisper40
Copy link
Author

I don't understand why the documentation is wrong, if a newbie comes to it it has to be right..
If we start basic sample with things that does not work, that makes crazy..

@iapicca
Copy link
iapicca commented May 16, 2021

@Whisper40
it wasn't always this way, at least not when I started, I'm sorry.

There is a silver lining,
what I didn't have was the excellent quality of youtube contents available today;
I recommend (in alphabetical order) :
Code With Andrea
Reso Coder
Robert Brunhage
Tadas Petra

@stuartmorgan
Copy link
Contributor

I don't understand why the documentation is wrong

The reason some sample code is wrong is that Dart had evolved over time, including introducing some breaking changes, so code that was correct when it was written does not always work in new versions of Dart.

@Whisper40
Copy link
Author

Flutter uses Dart, so yes the official documentation has to follow change.
I don't know if the documentation is opensource but it should be to avoid this type of problem.

@stuartmorgan
Copy link
Contributor

Flutter uses Dart, so yes the official documentation has to follow change.

I'm not saying it shouldn't. I'm saying that in practice, documentation gets written at a point in time, and can become incorrect, and then someone needs to a) discover the places that are no longer correct, and b) fix them, and that process is not automatic or foolproof. (I have filled an issue to track a possible approach to automate (a)).

I don't know if the documentation is opensource

Yes: https://github.com/flutter/website

but it should be to avoid this type of problem.

Being open source does not mean it is error-proof or self-updating.

@iapicca
Copy link
iapicca commented May 16, 2021

@stuartmorgan

I have filled an issue to track a possible approach to automate

this is awesome

please consider this pr (@Whisper40 you may try it if you like)

I'm not good with words but I think a possible "misstep" for beginners
is to run the sample for web and getting yelled at because of cors
maybe would be a good idea to add a disclaimer or something

@Whisper40
Copy link
Author
Whisper40 commented May 16, 2021

@iapicca I'm trying your PR, it compiles without errors now, but i have an error with the parsePhotos ( in my case parsePosts). The loader is displayed but then it freezes and throw the icon exception
I/flutter (12255): Invalid argument(s): Illegal argument in isolate message : (object is a closure - Function 'parsePosts':.)
I don't know if i have do something wrong, i have exactly the same parse method :

  List<Post> parsePosts(String responseBody) {
    final parsed = jsonDecode(responseBody) as List;
    return [for (final json in parsed) Post.fromJson(json)];
  }

@iapicca
Copy link
iapicca commented May 16, 2021

I'm trying your PR, it compiles without errors now,

good

but i have an error with the parsePhotos ( in my case parsePosts). The loader is displayed but then it freezes and throw the icon exception
I/flutter (12255): Invalid argument(s): Illegal argument in isolate message : (object is a closure - Function 'parsePosts':.)
I don't know if i have do something wrong, i have exactly the same parse method :

  List<Post> parsePosts(String responseBody) {
    final parsed = jsonDecode(responseBody) as List;
    return [for (final json in parsed) Post.fromJson(json)];
  }

@Whisper40
can you show me your full code sample and the json you are decoding,
I don't see the Post class here and it isn't in the cookbook

also make sure you are using the latest stable or above (maybe avoid beta it's super buggy, prefer dev or even master)

@Whisper40
Copy link
Author
Whisper40 commented May 16, 2021

Json here : https://jsonplaceholder.typicode.com/posts

$ flutter upgrade
Flutter is already up to date on channel stable
Flutter 2.0.6 • channel stable • https://github.com/flutter/flutter.git  
Framework • revision 1d9032c7e1 (2 weeks ago) • 2021-04-29 17:37:58 -0700
Engine • revision 05e680e202
Tools • Dart 2.12.3

http_service.dart :

import 'dart:convert';
import 'package:flutter/foundation.dart';
import 'package:http/http.dart' as http;
import '../models/post_model.dart';

class HttpService {
  
  // final uri = Uri.parse('https://jsonplaceholder.typicode.com') as String;
  // final String postsURL = '/posts';
  // final Map<String, String> headers = {
  //   'Content-Type': 'application/json'
  // };

  List<Post> parsePosts(String responseBody) {
    final parsed = jsonDecode(responseBody) as List;
    return [for (final json in parsed) Post.fromJson(json)];
  }

  Future<List<Post>> fetchPosts(http.Client client) async {
    final uri = Uri.parse('https://jsonplaceholder.typicode.com/posts');
    final response = await client.get(uri);

    if (response.statusCode == 200) {
      return compute(parsePosts,response.body);
    } else {
      throw Exception("Failed to load posts ${response.statusCode}");
    }
  }
}

posts_page.dart :

import 'package:flutter/material.dart';
import '../models/post_model.dart';
import '../services/http_service.dart';
import 'package:http/http.dart' as http;

class PostsPage extends StatelessWidget {
  final String title;
  final HttpService httpService = HttpService();
  PostsPage({Key? key, required this.title}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(title),
      ),
      body: FutureBuilder<List<Post>>(
        future: httpService.fetchPosts(http.Client()),
        builder: (context, snapshot) {
          if (snapshot.hasError) {
            print(snapshot.error);
            return const Center(child: Icon(Icons.error));
          }
          return snapshot.hasData
              ? PostsList(posts: snapshot.data!)
              : const Center(child: CircularProgressIndicator());
        },
      ),
    );
  }
}

class PostsList extends StatelessWidget {
  final List<Post> posts;

  const PostsList({Key? key, required this.posts}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return GridView.builder(
      gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
        crossAxisCount: 2,
      ),
      itemCount: posts.length,
      itemBuilder: (context, index) {
        return Image.network(posts[index].title);
      },
    );
  }
}

post_model.dart :

import 'package:flutter/foundation.dart';
import 'package:json_annotation/json_annotation.dart';

part 'post_model.g.dart';

@JsonSerializable()
@immutable
class Post {

  const Post({
    required this.userId,
    required this.id,
    required this.title,
    required this.body,
  });

  factory Post.fromJson(Map<String, dynamic?> json) => _$PostFromJson(json);
  Map<String, dynamic> toJson() => _$PostToJson(this);

  final int userId, id;
  final String title, body;
 
}

@iapicca
Copy link
iapicca commented May 16, 2021

@Whisper40
a couple of error in your code

  • trivial error... your PostsList expect an url for an image, but you Post doesn't have it
  • VERY IMPORTANT error: as for the error logs you provided, remember
    isolates accept only 2 functions: top level or static, while yours was neither

here is your code working

fixed code
import 'dart:convert';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:http/http.dart' as http;

void main() => runApp(const MaterialApp(home: PostsPage(title: 'title')));

class HttpService {
  static List<Post> _parsePosts(String responseBody) {
    final parsed = jsonDecode(responseBody) as List;
    return [for (final json in parsed) Post.fromJson(json)];
  }

  static Future<List<Post>> fetchPosts(http.Client client) async {
    final uri = Uri.parse('https://jsonplaceholder.typicode.com/posts');
    final response = await client.get(uri);

    if (response.statusCode == 200) {
      return compute(_parsePosts, response.body);
    } else {
      throw Exception("Failed to load posts ${response.statusCode}");
    }
  }
}

@immutable
class Post {
  const Post({
    required this.userId,
    required this.id,
    required this.title,
    required this.body,
  });

  factory Post.fromJson(Map<String, Object?> json) {
    return Post(
      body: json['body'] as String,
      id: json['id'] as int,
      userId: json['userId'] as int,
      title: json['title'] as String,
    );
  }

  final int userId, id;
  final String title, body;
}

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

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(title),
      ),
      body: FutureBuilder<List<Post>>(
        future: HttpService.fetchPosts(http.Client()),
        builder: (context, snapshot) {
          if (snapshot.hasError) {
            print(snapshot.error);
            return const Center(child: Icon(Icons.error));
          }
          return snapshot.hasData
              ? PostsList(posts: snapshot.data!)
              : const Center(child: CircularProgressIndicator());
        },
      ),
    );
  }
}

class PostsList extends StatelessWidget {
  final List<Post> posts;

  const PostsList({Key? key, required this.posts}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return GridView.builder(
      gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
        crossAxisCount: 2,
      ),
      itemCount: posts.length,
      itemBuilder: (context, index) {
        return Card(
          child: Column(
            children: [
              Center(
                child: Text(posts[index].title),
              ),
              Center(
                child: Text(posts[index].body),
              )
            ],
          ),
        );
      },
    );
  }
}

bonus...

  • there are 2 things I don't like in this world: paprika & codegen
    if you really like codegen I strongly recommend freezed
  • Map<String,Object?> > Map<String,dynamic>

@Whisper40
Copy link
Author

Thanks for your correction ! Now it works, i will take for sure a look at freezed !
With this error "A RenderFlex overflowed by 10 pixels on the bottom." i know now that i'm on mobile dev haha :)
Thanks for your help !

@iapicca
Copy link
iapicca commented May 16, 2021

@Whisper40
would you mind to keep the issue open until the pr lands

@Whisper40 Whisper40 reopened this May 16, 2021
@TahaTesser TahaTesser added st.triage.triage-team Triage team reviewing and categorizing the issue d: cookbook act.await-dev-pr Needs dev PR to merge before merging docs and removed st.triage.triage-team Triage team reviewing and categorizing the issue labels May 17, 2021
@iapicca
Copy link
iapicca commented Jun 6, 2021

sorry guys I gave up on the PR,
I'm shocked by the amount of overhead needed for a simple pr on flutter/website
but this explains @Whisper40 observation

Flutter uses Dart, so yes the official documentation has to follow change.
I don't know if the documentation is opensource but it should be to avoid this type of problem.

imho the reason is

nobody submits pr to update code samples and such because it's a very bad experience

@darshankawar
Copy link
Member

Transferring this issue to website repo, as this is more likely to be address there.

@darshankawar darshankawar transferred this issue from flutter/flutter Feb 1, 2022
@danagbemava-nc danagbemava-nc added a.cookbook Relates to a cookbook recipe or guide e1-hours Effort: < 8 hrs p2-medium Necessary but not urgent concern. Resolve when possible. and removed d: cookbook act.await-dev-pr Needs dev PR to merge before merging docs labels Feb 2, 2022
@atsansone atsansone added the lang.json Involves JSON formatted data label Jun 2, 2023
@atsansone atsansone changed the title Official flutter documentation with Json Serialization doesn't work Fix example on 'Parse JSON in the background' page Jun 5, 2023
@atsansone atsansone added act.question Relates to issues that writers need SME help st.blocked Issue cannot continue until another action completes labels Jun 5, 2023
@atsansone
Copy link
Contributor

This recipe appears to have most of the changes suggested in your initial PR. The example shown works. Closing this issue. If this is still a concern, please open a new issue and refer back to this one. Thanks!

@atsansone atsansone added ltw-triage cl.fixed Issue is closed as fixed and removed act.question Relates to issues that writers need SME help st.blocked Issue cannot continue until another action completes labels Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a.cookbook Relates to a cookbook recipe or guide cl.fixed Issue is closed as fixed e1-hours Effort: < 8 hrs lang.json Involves JSON formatted data p2-medium Necessary but not urgent concern. Resolve when possible.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants