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: identify dependencies that might require the --no-tree-shake-icons flag #7724

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

Conversation

emwp
Copy link
Contributor

@emwp emwp commented Sep 24, 2024

Description

Related to this issue: #6197
Trying to identify dependencies that might require the --no-tree-shake-icons build flag.

Unfortunately can not allow users to pass arbitrary/custom flags when running the deploy function, so tried to identify dependencies that are likely to require the flag to build.

Scenarios Tested

  • Building with dependencies that would add the flag to the build command.
  • Building with no dependencies that would add the flag to the build command.

fixes #6197

src/frameworks/flutter/index.ts Outdated Show resolved Hide resolved
Comment on lines 15 to 18
const pubSpecPath = "./pubspec.yaml";
let pubSpec: Record<string, any> = {};
try {
const pubSpecBuffer = await readFile(pubSpecPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use a path relative to the project root as the project can be located in a subdir, as we do in discover

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());

@@ -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<string> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make it generic like getAdditionalBuildArgs and make it return an array of strings, then we can accommodate more use cases in the future if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not able to add custom paramters to flutter build in firebase hosting
2 participants