From e7803a0f45527448b65de19ee1b26f02a43c63a3 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Tue, 24 Sep 2024 10:59:25 -0300 Subject: [PATCH 1/9] fix: identify dependencies that might require the --no-tree-shake-icons flag to build --- src/frameworks/flutter/index.ts | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/frameworks/flutter/index.ts b/src/frameworks/flutter/index.ts index 283a737d659..711dc859235 100644 --- a/src/frameworks/flutter/index.ts +++ b/src/frameworks/flutter/index.ts @@ -8,6 +8,7 @@ import { BuildResult, Discovery, FrameworkType, SupportLevel } from "../interfac import { FirebaseError } from "../../error"; import { assertFlutterCliExists } from "./utils"; import { DART_RESERVED_WORDS, FALLBACK_PROJECT_NAME } from "./constants"; +import { logger } from "../.."; export const name = "Flutter Web"; export const type = FrameworkType.Framework; @@ -50,9 +51,35 @@ 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 pubSpecPath = "./pubspec.yaml"; + let pubSpec: Record = {}; + try { + const pubSpecBuffer = await readFile(pubSpecPath); + pubSpec = yaml.parse(pubSpecBuffer.toString()); + } catch (error) { + logger.info("pubspec.yaml not found, skipping tree shaking"); + } + + 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 buildArgs = ["build", "web"]; + if (hasTreeShakePackage) { + buildArgs.push("--no-tree-shake-icons"); + } + + 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 }); } From 0781a33212962a41cf2b78b79480e571c36eb8f9 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Tue, 24 Sep 2024 11:44:09 -0300 Subject: [PATCH 2/9] chore: add test --- src/frameworks/flutter/index.ts | 29 ++------------- src/frameworks/flutter/utils.spec.ts | 55 ++++++++++++++++++++++++++-- src/frameworks/flutter/utils.ts | 26 +++++++++++++ 3 files changed, 81 insertions(+), 29 deletions(-) diff --git a/src/frameworks/flutter/index.ts b/src/frameworks/flutter/index.ts index 711dc859235..8c846f2a01e 100644 --- a/src/frameworks/flutter/index.ts +++ b/src/frameworks/flutter/index.ts @@ -6,9 +6,8 @@ import { readFile } from "fs/promises"; import { BuildResult, Discovery, FrameworkType, SupportLevel } from "../interfaces"; import { FirebaseError } from "../../error"; -import { assertFlutterCliExists } from "./utils"; +import { assertFlutterCliExists, getTreeShakeFlag } from "./utils"; import { DART_RESERVED_WORDS, FALLBACK_PROJECT_NAME } from "./constants"; -import { logger } from "../.."; export const name = "Flutter Web"; export const type = FrameworkType.Framework; @@ -54,30 +53,8 @@ export function init(setup: any, config: any) { export async function build(cwd: string): Promise { assertFlutterCliExists(); - const pubSpecPath = "./pubspec.yaml"; - let pubSpec: Record = {}; - try { - const pubSpecBuffer = await readFile(pubSpecPath); - pubSpec = yaml.parse(pubSpecBuffer.toString()); - } catch (error) { - logger.info("pubspec.yaml not found, skipping tree shaking"); - } - - 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 buildArgs = ["build", "web"]; - if (hasTreeShakePackage) { - buildArgs.push("--no-tree-shake-icons"); - } + const otherArgs = await getTreeShakeFlag(); + 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"); diff --git a/src/frameworks/flutter/utils.spec.ts b/src/frameworks/flutter/utils.spec.ts index 2635b8fa14a..5b929e79b2b 100644 --- a/src/frameworks/flutter/utils.spec.ts +++ b/src/frameworks/flutter/utils.spec.ts @@ -3,8 +3,9 @@ import * as sinon from "sinon"; import { EventEmitter } from "events"; import { Writable } from "stream"; import * as crossSpawn from "cross-spawn"; - -import { assertFlutterCliExists } from "./utils"; +import * as yaml from "yaml"; +import { assertFlutterCliExists, getTreeShakeFlag } from "./utils"; +import * as fs from "fs"; describe("Flutter utils", () => { describe("assertFlutterCliExists", () => { @@ -25,10 +26,58 @@ 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("getTreeShakeFlag", () => { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should return '--no-tree-shake-icons' if a tree shake package is found", async () => { + const pubSpecContent = ` + dependencies: + material_icons_named: any + `; + const readFileStub = sandbox.stub(fs, "readFile").resolves(Buffer.from(pubSpecContent)); + const yamlParseStub = sandbox.stub(yaml, "parse").returns({ + dependencies: { + material_icons_named: "any", + }, + }); + + const result = await getTreeShakeFlag(); + expect(result).to.equal("--no-tree-shake-icons"); + sinon.assert.calledOnce(readFileStub); + sinon.assert.calledOnce(yamlParseStub); + }); + + it("should return an empty string if no tree shake package is found", async () => { + const pubSpecContent = ` + dependencies: + some_other_package: any + `; + const readFileStub = sandbox.stub(fs, "readFile").resolves(Buffer.from(pubSpecContent)); + const yamlParseStub = sandbox.stub(yaml, "parse").returns({ + dependencies: { + some_other_package: "any", + }, + }); + + const result = await getTreeShakeFlag(); + expect(result).to.equal(""); + sinon.assert.calledOnce(readFileStub); + sinon.assert.calledOnce(yamlParseStub); + }); + }); }); diff --git a/src/frameworks/flutter/utils.ts b/src/frameworks/flutter/utils.ts index 422cd6cc6e8..2479d4976a3 100644 --- a/src/frameworks/flutter/utils.ts +++ b/src/frameworks/flutter/utils.ts @@ -1,5 +1,7 @@ import { sync as spawnSync } from "cross-spawn"; import { FirebaseError } from "../../error"; +import { readFile } from "fs/promises"; +import * as yaml from "yaml"; export function assertFlutterCliExists() { const process = spawnSync("flutter", ["--version"], { stdio: "ignore" }); @@ -8,3 +10,27 @@ export function assertFlutterCliExists() { "Flutter CLI not found, follow the instructions here https://docs.flutter.dev/get-started/install before trying again.", ); } + +export async function getTreeShakeFlag(): Promise { + const pubSpecPath = "./pubspec.yaml"; + let pubSpec: Record = {}; + try { + const pubSpecBuffer = await readFile(pubSpecPath); + pubSpec = yaml.parse(pubSpecBuffer.toString()); + } catch (error) { + console.info("pubspec.yaml not found, skipping tree shaking"); + return ""; + } + + 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]); + return hasTreeShakePackage ? "--no-tree-shake-icons" : ""; +} From ed84b66cc30bc8024f61ddc177a30760bafb0179 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Mon, 30 Sep 2024 22:16:21 -0300 Subject: [PATCH 3/9] fix: refactor util functiosn --- src/frameworks/flutter/index.ts | 5 +- src/frameworks/flutter/utils.spec.ts | 75 ++++++++++++++++------------ src/frameworks/flutter/utils.ts | 23 +++++---- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/frameworks/flutter/index.ts b/src/frameworks/flutter/index.ts index 8c846f2a01e..f4df53f42df 100644 --- a/src/frameworks/flutter/index.ts +++ b/src/frameworks/flutter/index.ts @@ -6,7 +6,7 @@ import { readFile } from "fs/promises"; import { BuildResult, Discovery, FrameworkType, SupportLevel } from "../interfaces"; import { FirebaseError } from "../../error"; -import { assertFlutterCliExists, getTreeShakeFlag } from "./utils"; +import { assertFlutterCliExists, getPubSpec, getTreeShakeFlag } from "./utils"; import { DART_RESERVED_WORDS, FALLBACK_PROJECT_NAME } from "./constants"; export const name = "Flutter Web"; @@ -53,7 +53,8 @@ export function init(setup: any, config: any) { export async function build(cwd: string): Promise { assertFlutterCliExists(); - const otherArgs = await getTreeShakeFlag(); + const pubSpec = await getPubSpec(cwd); + const otherArgs = getTreeShakeFlag(pubSpec); const buildArgs = ["build", "web", otherArgs].filter(Boolean); const build = spawnSync("flutter", buildArgs, { cwd, stdio: "inherit" }); diff --git a/src/frameworks/flutter/utils.spec.ts b/src/frameworks/flutter/utils.spec.ts index 5b929e79b2b..eaa14ce913c 100644 --- a/src/frameworks/flutter/utils.spec.ts +++ b/src/frameworks/flutter/utils.spec.ts @@ -3,9 +3,9 @@ import * as sinon from "sinon"; import { EventEmitter } from "events"; import { Writable } from "stream"; import * as crossSpawn from "cross-spawn"; -import * as yaml from "yaml"; -import { assertFlutterCliExists, getTreeShakeFlag } from "./utils"; -import * as fs from "fs"; +import { assertFlutterCliExists, getTreeShakeFlag, getPubSpec } from "./utils"; +import * as fs from "fs/promises"; +import * as path from "path"; describe("Flutter utils", () => { describe("assertFlutterCliExists", () => { @@ -34,6 +34,31 @@ describe("Flutter utils", () => { }); describe("getTreeShakeFlag", () => { + it("should return '--no-tree-shake-icons' when a tree-shake package is present", () => { + const pubSpec = { + dependencies: { + material_icons_named: "^1.0.0", + }, + }; + expect(getTreeShakeFlag(pubSpec)).to.equal("--no-tree-shake-icons"); + }); + + it("should return an empty string when no tree-shake package is present", () => { + const pubSpec = { + dependencies: { + some_other_package: "^1.0.0", + }, + }; + expect(getTreeShakeFlag(pubSpec)).to.equal(""); + }); + + it("should return an empty string when dependencies is undefined", () => { + const pubSpec = {}; + expect(getTreeShakeFlag(pubSpec)).to.equal(""); + }); + }); + + describe("getPubSpec", () => { let sandbox: sinon.SinonSandbox; beforeEach(() => { @@ -44,40 +69,28 @@ describe("Flutter utils", () => { sandbox.restore(); }); - it("should return '--no-tree-shake-icons' if a tree shake package is found", async () => { - const pubSpecContent = ` - dependencies: - material_icons_named: any - `; - const readFileStub = sandbox.stub(fs, "readFile").resolves(Buffer.from(pubSpecContent)); - const yamlParseStub = sandbox.stub(yaml, "parse").returns({ + it("should read and parse pubspec.yaml file", async () => { + const mockYamlContent = "dependencies:\n flutter:\n sdk: flutter\n some_package: ^1.0.0"; + const expectedParsedYaml = { dependencies: { - material_icons_named: "any", + flutter: { sdk: "flutter" }, + some_package: "^1.0.0", }, - }); + }; - const result = await getTreeShakeFlag(); - expect(result).to.equal("--no-tree-shake-icons"); - sinon.assert.calledOnce(readFileStub); - sinon.assert.calledOnce(yamlParseStub); + sandbox.stub(fs, "readFile").resolves(Buffer.from(mockYamlContent)); + sandbox.stub(path, "join").returns("/path/pubspec.yaml"); + + const result = await getPubSpec("/path"); + expect(result).to.deep.equal(expectedParsedYaml); }); - it("should return an empty string if no tree shake package is found", async () => { - const pubSpecContent = ` - dependencies: - some_other_package: any - `; - const readFileStub = sandbox.stub(fs, "readFile").resolves(Buffer.from(pubSpecContent)); - const yamlParseStub = sandbox.stub(yaml, "parse").returns({ - dependencies: { - some_other_package: "any", - }, - }); + it("should return an empty object if pubspec.yaml is not found", async () => { + sandbox.stub(fs, "readFile").rejects(new Error("File not found")); + sandbox.stub(console, "info").returns(); // Suppress console output - const result = await getTreeShakeFlag(); - expect(result).to.equal(""); - sinon.assert.calledOnce(readFileStub); - sinon.assert.calledOnce(yamlParseStub); + const result = await getPubSpec("/path"); + expect(result).to.deep.equal({}); }); }); }); diff --git a/src/frameworks/flutter/utils.ts b/src/frameworks/flutter/utils.ts index 2479d4976a3..97217c448ca 100644 --- a/src/frameworks/flutter/utils.ts +++ b/src/frameworks/flutter/utils.ts @@ -1,6 +1,7 @@ import { sync as spawnSync } from "cross-spawn"; import { FirebaseError } from "../../error"; import { readFile } from "fs/promises"; +import { join } from "path"; import * as yaml from "yaml"; export function assertFlutterCliExists() { @@ -11,17 +12,7 @@ export function assertFlutterCliExists() { ); } -export async function getTreeShakeFlag(): Promise { - const pubSpecPath = "./pubspec.yaml"; - let pubSpec: Record = {}; - try { - const pubSpecBuffer = await readFile(pubSpecPath); - pubSpec = yaml.parse(pubSpecBuffer.toString()); - } catch (error) { - console.info("pubspec.yaml not found, skipping tree shaking"); - return ""; - } - +export function getTreeShakeFlag(pubSpec: Record): string { const treeShakePackages = [ "material_icons_named", "material_symbols_icons", @@ -34,3 +25,13 @@ export async function getTreeShakeFlag(): Promise { const hasTreeShakePackage = treeShakePackages.some((pkg) => pubSpec.dependencies?.[pkg]); return hasTreeShakePackage ? "--no-tree-shake-icons" : ""; } + +export async function getPubSpec(cwd: string): Promise> { + try { + const pubSpecBuffer = await readFile(join(cwd, "pubspec.yaml")); + return yaml.parse(pubSpecBuffer.toString()); + } catch (error) { + console.info("pubspec.yaml not found, skipping tree shaking"); + return {}; + } +} From 042009457d4e3365e6e670f78fffa619e4d732e8 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Mon, 30 Sep 2024 22:47:39 -0300 Subject: [PATCH 4/9] fix: refactor flutter web util functions --- src/frameworks/flutter/index.ts | 8 ++--- src/frameworks/flutter/utils.spec.ts | 46 ++++++++++++++++++++-------- src/frameworks/flutter/utils.ts | 38 ++++++++++++++++++++--- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/frameworks/flutter/index.ts b/src/frameworks/flutter/index.ts index f4df53f42df..e6e95a60ffd 100644 --- a/src/frameworks/flutter/index.ts +++ b/src/frameworks/flutter/index.ts @@ -6,7 +6,7 @@ import { readFile } from "fs/promises"; import { BuildResult, Discovery, FrameworkType, SupportLevel } from "../interfaces"; import { FirebaseError } from "../../error"; -import { assertFlutterCliExists, getPubSpec, getTreeShakeFlag } from "./utils"; +import { assertFlutterCliExists, getPubSpec, getAdditionalBuildArgs } from "./utils"; import { DART_RESERVED_WORDS, FALLBACK_PROJECT_NAME } from "./constants"; export const name = "Flutter Web"; @@ -50,12 +50,12 @@ export function init(setup: any, config: any) { return Promise.resolve(); } -export async function build(cwd: string): Promise { +export async function build(cwd: string): Promise { assertFlutterCliExists(); const pubSpec = await getPubSpec(cwd); - const otherArgs = getTreeShakeFlag(pubSpec); - const buildArgs = ["build", "web", otherArgs].filter(Boolean); + 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"); diff --git a/src/frameworks/flutter/utils.spec.ts b/src/frameworks/flutter/utils.spec.ts index eaa14ce913c..bdefec15d07 100644 --- a/src/frameworks/flutter/utils.spec.ts +++ b/src/frameworks/flutter/utils.spec.ts @@ -3,9 +3,10 @@ import * as sinon from "sinon"; import { EventEmitter } from "events"; import { Writable } from "stream"; import * as crossSpawn from "cross-spawn"; -import { assertFlutterCliExists, getTreeShakeFlag, getPubSpec } 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", () => { @@ -33,28 +34,28 @@ describe("Flutter utils", () => { }); }); - describe("getTreeShakeFlag", () => { + 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(getTreeShakeFlag(pubSpec)).to.equal("--no-tree-shake-icons"); + expect(getAdditionalBuildArgs(pubSpec)).to.deep.equal(["--no-tree-shake-icons"]); }); - it("should return an empty string when no tree-shake package is present", () => { + it("should return an empty array when no tree-shake package is present", () => { const pubSpec = { dependencies: { some_other_package: "^1.0.0", }, }; - expect(getTreeShakeFlag(pubSpec)).to.equal(""); + expect(getAdditionalBuildArgs(pubSpec)).to.deep.equal([]); }); - it("should return an empty string when dependencies is undefined", () => { + it("should return an empty array when dependencies is undefined", () => { const pubSpec = {}; - expect(getTreeShakeFlag(pubSpec)).to.equal(""); + expect(getAdditionalBuildArgs(pubSpec)).to.deep.equal([]); }); }); @@ -69,7 +70,7 @@ describe("Flutter utils", () => { sandbox.restore(); }); - it("should read and parse pubspec.yaml file", async () => { + 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: { @@ -78,19 +79,40 @@ describe("Flutter utils", () => { }, }; + sandbox.stub(fsExtra, "pathExists").resolves(true); sandbox.stub(fs, "readFile").resolves(Buffer.from(mockYamlContent)); - sandbox.stub(path, "join").returns("/path/pubspec.yaml"); + 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 is not found", async () => { - sandbox.stub(fs, "readFile").rejects(new Error("File not found")); - sandbox.stub(console, "info").returns(); // Suppress console output + 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 97217c448ca..4117e1f6ddf 100644 --- a/src/frameworks/flutter/utils.ts +++ b/src/frameworks/flutter/utils.ts @@ -1,6 +1,7 @@ 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"; @@ -12,7 +13,19 @@ export function assertFlutterCliExists() { ); } -export function getTreeShakeFlag(pubSpec: Record): string { +/** + * 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[] { const treeShakePackages = [ "material_icons_named", "material_symbols_icons", @@ -23,15 +36,30 @@ export function getTreeShakeFlag(pubSpec: Record): string { ]; const hasTreeShakePackage = treeShakePackages.some((pkg) => pubSpec.dependencies?.[pkg]); - return hasTreeShakePackage ? "--no-tree-shake-icons" : ""; + const treeShakeFlags = hasTreeShakePackage ? ["--no-tree-shake-icons"] : []; + return [...treeShakeFlags]; } -export async function getPubSpec(cwd: string): Promise> { +/** + * 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(cwd, "pubspec.yaml")); + const pubSpecBuffer = await readFile(join(dir, "pubspec.yaml")); return yaml.parse(pubSpecBuffer.toString()); } catch (error) { - console.info("pubspec.yaml not found, skipping tree shaking"); + console.info("Failed to read pubspec.yaml"); return {}; } } From 14c93b4e4011282e4d2bb9668a45b9e535bfa834 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Mon, 30 Sep 2024 22:51:16 -0300 Subject: [PATCH 5/9] fix: add comment/documentation --- src/frameworks/flutter/utils.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/frameworks/flutter/utils.ts b/src/frameworks/flutter/utils.ts index 4117e1f6ddf..c3cff5abb55 100644 --- a/src/frameworks/flutter/utils.ts +++ b/src/frameworks/flutter/utils.ts @@ -26,6 +26,12 @@ export function assertFlutterCliExists() { * 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", From b998de954262c2ba780114d593c0580544e4bfca Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Tue, 8 Oct 2024 14:26:54 -0300 Subject: [PATCH 6/9] chore: changelog --- CHANGELOG.md | 1 + src/frameworks/flutter/index.ts | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 328b52d1df8..72295b49dc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1 +1,2 @@ - Added App Hosting as an option for firebase init. (#7803) +- Parsing pubspec.yaml to detect dependencies 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 e6e95a60ffd..5d7fcb59a8b 100644 --- a/src/frameworks/flutter/index.ts +++ b/src/frameworks/flutter/index.ts @@ -1,8 +1,6 @@ import { sync as spawnSync } from "cross-spawn"; -import { copy, pathExists } from "fs-extra"; +import { copy } 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"; @@ -14,10 +12,7 @@ export const type = FrameworkType.Framework; 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 }; From be253014ef9a264c8d4df8a85148572982df6cfd Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Tue, 8 Oct 2024 14:43:40 -0300 Subject: [PATCH 7/9] fix: discover function return --- src/frameworks/flutter/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/frameworks/flutter/index.ts b/src/frameworks/flutter/index.ts index 5d7fcb59a8b..3ab873842e2 100644 --- a/src/frameworks/flutter/index.ts +++ b/src/frameworks/flutter/index.ts @@ -1,5 +1,5 @@ import { sync as spawnSync } from "cross-spawn"; -import { copy } from "fs-extra"; +import { copy, pathExists } from "fs-extra"; import { join } from "path"; import { BuildResult, Discovery, FrameworkType, SupportLevel } from "../interfaces"; @@ -12,6 +12,8 @@ export const type = FrameworkType.Framework; 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 pubSpec = await getPubSpec(dir); const usingFlutter = pubSpec.dependencies?.flutter; if (!usingFlutter) return; From dbe3afc6228fc7665156ee2628dc43636b719873 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Thu, 10 Oct 2024 19:24:53 -0300 Subject: [PATCH 8/9] Update CHANGELOG.md Co-authored-by: Leonardo Ortiz --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72295b49dc5..70bf8073e29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,2 +1,2 @@ - Added App Hosting as an option for firebase init. (#7803) -- Parsing pubspec.yaml to detect dependencies that might require the --no-tree-shake-icons flag in order to build. (#7724) +- Fixed Flutter web apps that might require the --no-tree-shake-icons flag in order to build. (#7724) From 0e769a468d3e09bdebc703c4c96a1821c79e8b14 Mon Sep 17 00:00:00 2001 From: Everton Pereira Date: Thu, 10 Oct 2024 19:40:25 -0300 Subject: [PATCH 9/9] Update CHANGELOG.md Co-authored-by: Leonardo Ortiz --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70bf8073e29..e3cb3cdaa3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,2 +1 @@ -- Added App Hosting as an option for firebase init. (#7803) - Fixed Flutter web apps that might require the --no-tree-shake-icons flag in order to build. (#7724)