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: webassembly #982

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

davidsdearaujo
Copy link
Member

@davidsdearaujo davidsdearaujo commented Jan 2, 2025

Problem

When you run an empty project in WASM using flutter_modular it breaks.

Solution

I believe this issue will be resolved in the Flutter engine itself in the future. It’s likely specific to WASM compilation, as it doesn’t occur on other platforms. Looks like some named params are not working well in WASM, so I recommend to use constructors with positional params of named params.

The injections causing the error in an empty project are internal to flutter_modular. I managed to resolve it by changing the constructors params from positional to named (see the changed files).

For developers

If you want to use named params and you are receiving this error you need to register your dependency like that (only for the dependencies that are returning error in the browser console):

🚫 Before (does not work on WASM)

    i.addSingleton<ModularRouteInformationParser>(ModularRouteInformationParser.new);

✅ After (Works on WASM)

    i.addSingleton<ModularRouteInformationParser>(() {
      return ModularRouteInformationParser(
        getArguments: i(),
        getRoute: i(),
        reportPush: i(),
        setArguments: i(),
        urlService: i(),
      );
    });

When we have it fixed (in Flutter side or in the auto_injector package) this workaround will not be needed anymore.

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Related Issues

Change named params to positional params in order to work with wasm
@joaovvrodrigues
Copy link

joaovvrodrigues commented Jan 19, 2025

@davidsdearaujo In the example of creating common Binds, it worked.
However, when we have a dependency that depends on another, we have another problem.

And this problem does not occur when we have WASM disabled.

Below is the example with an import of another class and the printscreen of the error.

Image

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

void main() {
  WidgetsFlutterBinding.ensureInitialized();
  runApp(ModularApp(
    module: AppModule(),
    child: const MyApp(),
  ));
}

class DioFake {
  DioFake();
}

class ChangeNotifierTest extends ChangeNotifier {
  int value = 0;
  final DioFake dioFake;

  ChangeNotifierTest({required this.dioFake});

  void increment() {
    value++;
    notifyListeners();
  }
}

class AppModule extends Module {
  @override
  void binds(i) {
    i.addSingleton(DioFake.new);
    i.addInstance(ChangeNotifierTest.new);
  }

  @override
  void routes(RouteManager r) {
    r.child("/", child: (context) => const MyHomePage(title: 'Flutter Demo Home Page'));
  }
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      title: 'Flutter Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      routerConfig: Modular.routerConfig,
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  ChangeNotifierTest test = Modular.get();

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text(
              'You have pushed the button this many times:',
            ),
            ListenableBuilder(
                listenable: test,
                builder: (context, _) {
                  return Text(
                    test.value.toString(),
                    style: Theme.of(context).textTheme.headlineMedium,
                  );
                }),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: test.increment,
        tooltip: 'Increment',
        child: const Icon(Icons.add),
      ),
    );
  }
}

@davidsdearaujo
Copy link
Member Author

davidsdearaujo commented Jan 21, 2025

@joaovvrodrigues
And this problem does not occur when we have WASM disabled.

As I explained in the PR description, by some reason dart is not working correctly when we try to infer named params to use YourClass.new. To avoid that problem you need to change the instances to use this structure:

For developers

If you want to use named params and you are receiving this error you need to register your dependency like that (only for the dependencies that are returning error in the browser console):

🚫 Before (does not work on WASM)

  i.addSingleton<ModularRouteInformationParser>(ModularRouteInformationParser.new);

✅ After (Works on WASM)

  i.addSingleton<ModularRouteInformationParser>(() {
    return ModularRouteInformationParser(
      getArguments: i(),
      getRoute: i(),
      reportPush: i(),
      setArguments: i(),
      urlService: i(),
    );
  });

When we have it fixed (in Flutter side or in the auto_injector package) this workaround will not be needed anymore.

In your case it would look like this:

class AppModule extends Module {
  @override
  void binds(i) {
    i.addSingleton(DioFake.new);
    i.addInstance(() => ChangeNotifierTest(dioFake: i()));
  }

  @override
  void routes(RouteManager r) {
    r.child("/", child: (context) => const MyHomePage(title: 'Flutter Demo Home Page'));
  }
}

Another way to avoid that problem is to change the way your class receive parameters, you can use positional params instead of named params. In this example you only need to change the constructor ChangeNotifierTest({required this.dioFake}); to ChangeNotifierTest(this.dioFake);:

class ChangeNotifierTest extends ChangeNotifier {
  int value = 0;
  final DioFake dioFake;

  ChangeNotifierTest(this.dioFake);

  void increment() {
    value++;
    notifyListeners();
  }
}

class AppModule extends Module {
  @override
  void binds(i) {
    i.addSingleton(DioFake.new);
    i.addInstance(ChangeNotifierTest.new);
  }

  @override
  void routes(RouteManager r) {
    r.child("/", child: (context) => const MyHomePage(title: 'Flutter Demo Home Page'));
  }
}

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

Successfully merging this pull request may close these issues.

BUG - Support for WebAssembly (Wasm) webassembly bug
2 participants