diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..e3cb3cdaa3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Fixed Flutter web apps that might require the --no-tree-shake-icons flag in order to build. (#7724) diff --git a/src/frameworks/flutter/index.ts b/src/frameworks/flutter/index.ts index 283a737d659..3ab873842e2 100644 --- a/src/frameworks/flutter/index.ts +++ b/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 { 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 { +export async function build(cwd: string): Promise { 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 }); } diff --git a/src/frameworks/flutter/utils.spec.ts b/src/frameworks/flutter/utils.spec.ts index 2635b8fa14a..bdefec15d07 100644 --- a/src/frameworks/flutter/utils.spec.ts +++ b/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; + }); + }); }); diff --git a/src/frameworks/flutter/utils.ts b/src/frameworks/flutter/utils.ts index 422cd6cc6e8..c3cff5abb55 100644 --- a/src/frameworks/flutter/utils.ts +++ b/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} 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[] { + /* + 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>} 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> { + 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 {}; + } +}