fix: identify dependencies that might require the --no-tree-shake-ico…

…ns flag (#7724)

* Fixed Flutter web apps that might require the --no-tree-shake-icons flag in order to build. 

Co-authored-by: Leonardo Ortiz <leo@monogram.io>

CHANGELOG.md

@@ -0,0 +1 @@
+- Fixed Flutter web apps that might require the --no-tree-shake-icons flag in order to build. (#7724)

src/frameworks/flutter/index.ts

@@ -1,12 +1,10 @@
 import { sync as spawnSync } from "cross-spawn";
 import { copy, pathExists } from "fs-extra";
 import { join } from "path";
-import * as yaml from "yaml";
-import { readFile } from "fs/promises";
 
 import { BuildResult, Discovery, FrameworkType, SupportLevel } from "../interfaces";
 import { FirebaseError } from "../../error";
-import { assertFlutterCliExists } from "./utils";
+import { assertFlutterCliExists, getPubSpec, getAdditionalBuildArgs } from "./utils";
 import { DART_RESERVED_WORDS, FALLBACK_PROJECT_NAME } from "./constants";
 
 export const name = "Flutter Web";
@@ -16,8 +14,7 @@ export const support = SupportLevel.Experimental;
 export async function discover(dir: string): Promise<Discovery | undefined> {
   if (!(await pathExists(join(dir, "pubspec.yaml")))) return;
   if (!(await pathExists(join(dir, "web")))) return;
-  const pubSpecBuffer = await readFile(join(dir, "pubspec.yaml"));
-  const pubSpec = yaml.parse(pubSpecBuffer.toString());
+  const pubSpec = await getPubSpec(dir);
   const usingFlutter = pubSpec.dependencies?.flutter;
   if (!usingFlutter) return;
   return { mayWantBackend: false };
@@ -50,9 +47,14 @@ export function init(setup: any, config: any) {
   return Promise.resolve();
 }
 
-export function build(cwd: string): Promise<BuildResult> {
+export async function build(cwd: string): Promise<BuildResult> {
   assertFlutterCliExists();
-  const build = spawnSync("flutter", ["build", "web"], { cwd, stdio: "inherit" });
+
+  const pubSpec = await getPubSpec(cwd);
+  const otherArgs = getAdditionalBuildArgs(pubSpec);
+  const buildArgs = ["build", "web", ...otherArgs].filter(Boolean);
+
+  const build = spawnSync("flutter", buildArgs, { cwd, stdio: "inherit" });
   if (build.status !== 0) throw new FirebaseError("Unable to build your Flutter app");
   return Promise.resolve({ wantsBackend: false });
 }

src/frameworks/flutter/utils.spec.ts

@@ -3,8 +3,10 @@ import * as sinon from "sinon";
 import { EventEmitter } from "events";
 import { Writable } from "stream";
 import * as crossSpawn from "cross-spawn";
-
-import { assertFlutterCliExists } from "./utils";
+import { assertFlutterCliExists, getAdditionalBuildArgs, getPubSpec } from "./utils";
+import * as fs from "fs/promises";
+import * as path from "path";
+import * as fsExtra from "fs-extra";
 
 describe("Flutter utils", () => {
   describe("assertFlutterCliExists", () => {
@@ -25,10 +27,92 @@ describe("Flutter utils", () => {
       process.stderr = new EventEmitter();
       process.status = 0;
 
-      const stub = sandbox.stub(crossSpawn, "sync").returns(process as any);
+      const stub = sandbox.stub(crossSpawn, "sync").returns(process);
 
       expect(assertFlutterCliExists()).to.be.undefined;
       sinon.assert.calledWith(stub, "flutter", ["--version"], { stdio: "ignore" });
     });
   });
+
+  describe("getAdditionalBuildArgs", () => {
+    it("should return '--no-tree-shake-icons' when a tree-shake package is present", () => {
+      const pubSpec = {
+        dependencies: {
+          material_icons_named: "^1.0.0",
+        },
+      };
+      expect(getAdditionalBuildArgs(pubSpec)).to.deep.equal(["--no-tree-shake-icons"]);
+    });
+
+    it("should return an empty array when no tree-shake package is present", () => {
+      const pubSpec = {
+        dependencies: {
+          some_other_package: "^1.0.0",
+        },
+      };
+      expect(getAdditionalBuildArgs(pubSpec)).to.deep.equal([]);
+    });
+
+    it("should return an empty array when dependencies is undefined", () => {
+      const pubSpec = {};
+      expect(getAdditionalBuildArgs(pubSpec)).to.deep.equal([]);
+    });
+  });
+
+  describe("getPubSpec", () => {
+    let sandbox: sinon.SinonSandbox;
+
+    beforeEach(() => {
+      sandbox = sinon.createSandbox();
+    });
+
+    afterEach(() => {
+      sandbox.restore();
+    });
+
+    it("should read and parse pubspec.yaml file when both pubspec.yaml and web directory exist", async () => {
+      const mockYamlContent = "dependencies:\n  flutter:\n    sdk: flutter\n  some_package: ^1.0.0";
+      const expectedParsedYaml = {
+        dependencies: {
+          flutter: { sdk: "flutter" },
+          some_package: "^1.0.0",
+        },
+      };
+
+      sandbox.stub(fsExtra, "pathExists").resolves(true);
+      sandbox.stub(fs, "readFile").resolves(Buffer.from(mockYamlContent));
+      sandbox.stub(path, "join").callsFake((...args) => args.join("/"));
+
+      const result = await getPubSpec("/path");
+      expect(result).to.deep.equal(expectedParsedYaml);
+    });
+
+    it("should return an empty object if pubspec.yaml doesn't exist", async () => {
+      const pathExistsStub = sandbox.stub(fsExtra, "pathExists");
+      pathExistsStub.withArgs("/path/pubspec.yaml", () => null).resolves(false);
+
+      const result = await getPubSpec("/path");
+      expect(result).to.deep.equal({});
+    });
+
+    it("should return an empty object if web directory doesn't exist", async () => {
+      const pathExistsStub = sandbox.stub(fsExtra, "pathExists");
+      pathExistsStub.withArgs("/path/pubspec.yaml", () => null).resolves(true);
+      pathExistsStub.withArgs("/path/web", () => null).resolves(false);
+
+      const result = await getPubSpec("/path");
+      expect(result).to.deep.equal({});
+    });
+
+    it("should return an empty object and log a message if there's an error reading pubspec.yaml", async () => {
+      sandbox.stub(fsExtra, "pathExists").resolves(true);
+      sandbox.stub(fs, "readFile").rejects(new Error("File read error"));
+      sandbox.stub(path, "join").callsFake((...args) => args.join("/"));
+      const consoleInfoStub = sandbox.stub(console, "info");
+
+      const result = await getPubSpec("/path");
+      expect(result).to.deep.equal({});
+      expect(consoleInfoStub.calledOnceWith("Failed to read pubspec.yaml")).to.be.true;
+    });
+  });
 });

src/frameworks/flutter/utils.ts

@@ -1,5 +1,9 @@
 import { sync as spawnSync } from "cross-spawn";
 import { FirebaseError } from "../../error";
+import { readFile } from "fs/promises";
+import { pathExists } from "fs-extra";
+import { join } from "path";
+import * as yaml from "yaml";
 
 export function assertFlutterCliExists() {
   const process = spawnSync("flutter", ["--version"], { stdio: "ignore" });
@@ -8,3 +12,60 @@ export function assertFlutterCliExists() {
       "Flutter CLI not found, follow the instructions here https://docs.flutter.dev/get-started/install before trying again.",
     );
 }
+
+/**
+ * Determines additional build arguments for Flutter based on the project's dependencies.
+ * @param {Record<string, any>} pubSpec - The parsed pubspec.yaml file contents.
+ * @return {string[]} An array of additional build arguments.
+ * @description
+ * This function checks if the project uses certain packages that might require additional
+ * flags to be added to the build step. If any of these packages are present in the
+ * project's dependencies, the function returns an array with these flags.
+ * Otherwise, it returns an empty array.
+ * This change is inspired from the following issue:
+ * https://github.com/firebase/firebase-tools/issues/6197
+ */
+export function getAdditionalBuildArgs(pubSpec: Record<string, any>): string[] {
+  /*
+  These packages are known to require the --no-tree-shake-icons flag
+  when building for web.
+  More dependencies might need to add here in the future.
+  Related issue: https://github.com/firebase/firebase-tools/issues/6197
+  */
+  const treeShakePackages = [
+    "material_icons_named",
+    "material_symbols_icons",
+    "material_design_icons_flutter",
+    "flutter_iconpicker",
+    "font_awesome_flutter",
+    "ionicons_named",
+  ];
+
+  const hasTreeShakePackage = treeShakePackages.some((pkg) => pubSpec.dependencies?.[pkg]);
+  const treeShakeFlags = hasTreeShakePackage ? ["--no-tree-shake-icons"] : [];
+  return [...treeShakeFlags];
+}
+
+/**
+ * Reads and parses the pubspec.yaml file from a given directory.
+ * @param {string} dir - The directory path where pubspec.yaml is located.
+ * @return {Promise<Record<string, any>>} A promise that resolves to the parsed contents of pubspec.yaml.
+ * @description
+ * This function checks for the existence of both pubspec.yaml and the 'web' directory
+ * in the given path. If either is missing, it returns an empty object.
+ * If both exist, it reads the pubspec.yaml file, parses its contents, and returns
+ * the parsed object. In case of any errors during this process, it logs a message
+ * and returns an empty object.
+ */
+export async function getPubSpec(dir: string): Promise<Record<string, any>> {
+  if (!(await pathExists(join(dir, "pubspec.yaml")))) return {};
+  if (!(await pathExists(join(dir, "web")))) return {};
+
+  try {
+    const pubSpecBuffer = await readFile(join(dir, "pubspec.yaml"));
+    return yaml.parse(pubSpecBuffer.toString());
+  } catch (error) {
+    console.info("Failed to read pubspec.yaml");
+    return {};
+  }
+}