diff --git a/docs/scripts/agent-instructions.test.ts b/docs/scripts/agent-instructions.test.ts new file mode 100644 index 00000000000000..d94534373bd1e3 --- /dev/null +++ b/docs/scripts/agent-instructions.test.ts @@ -0,0 +1,17 @@ +import { buildFeedbackSection } from './agent-instructions.ts'; + +describe('buildFeedbackSection', () => { + it('prints the Expo feedback CLI command and curl fallback with the page URL', () => { + expect(buildFeedbackSection('/skills/')).toBe(`## Submitting Feedback + +If you encounter errors, misleading or outdated information, report it so Expo can be improved: + +Preferred command: +npx --yes submit-expo-feedback@latest --category docs --subject "/skills/" "" + +Direct HTTP fallback: +curl -X POST https://api.expo.dev/v2/feedback/docs-send -H 'Content-Type: application/json' -d '{"url":"/skills/","feedback":"🤖 Agent feedback for docs: (, )"}' + +Only submit when you have something specific and actionable to report. Try to give the most context.`); + }); +}); diff --git a/docs/scripts/agent-instructions.ts b/docs/scripts/agent-instructions.ts index 2dd5621943b28b..81bdc7d946c30a 100644 --- a/docs/scripts/agent-instructions.ts +++ b/docs/scripts/agent-instructions.ts @@ -2,10 +2,8 @@ * Agent feedback instructions appended into per-page .md files. * * Each page's generated markdown includes an block (placed - * right after the frontmatter) telling LLM-based agents how to POST feedback to - * the existing /v2/feedback/docs-send endpoint. The `🤖 Agent feedback:` prefix - * inside the feedback string badges the Slack message in #docs-feedback-feed - * without requiring a backend change. + * right after the frontmatter) telling LLM-based agents how to submit feedback + * with the Expo feedback CLI or direct HTTP fallback. * * Intentionally isolated in its own module so the experiment can be tweaked or * removed without touching the broader markdown generation utilities. @@ -29,11 +27,15 @@ export function shouldAppendAgentInstructions(markdown: string): boolean { export function buildFeedbackSection(pathname: string): string { return `## Submitting Feedback -If this page contains errors, outdated information, or gaps that blocked you from completing a task, report it so the docs can be improved: +If you encounter errors, misleading or outdated information, report it so Expo can be improved: +Preferred command: +npx --yes submit-expo-feedback@latest --category docs --subject "${pathname}" "" + +Direct HTTP fallback: curl -X POST https://api.expo.dev/v2/feedback/docs-send -H 'Content-Type: application/json' -d '{"url":"${pathname}","feedback":"🤖 Agent feedback for docs: (, )"}' -Only submit when you have something specific and actionable to report. Replace with the model you are running as and with the tool you are running in (for example, Claude Code, Cursor, Codex CLI).`; +Only submit when you have something specific and actionable to report. Try to give the most context.`; } export function wrapAgentInstructions(sections: string[]): string { diff --git a/docs/scripts/generate-llms/shared.test.js b/docs/scripts/generate-llms/shared.test.js index fea8d6319ea617..8599bd1ed42ec6 100644 --- a/docs/scripts/generate-llms/shared.test.js +++ b/docs/scripts/generate-llms/shared.test.js @@ -121,7 +121,19 @@ describe('stripAgentInstructions', () => { '', '## Submitting Feedback', '', - 'curl -X POST https://api.expo.dev/v2/feedback/docs-send', + 'Preferred command:', + [ + 'npx --yes submit-expo-feedback@latest --category docs', + '--subject "/versions/latest/sdk/camera/"', + '""', + ].join(' '), + '', + 'Direct HTTP fallback:', + [ + 'curl -X POST https://api.expo.dev/v2/feedback/docs-send', + "-H 'Content-Type: application/json'", + `-d '{"url":"/versions/latest/sdk/camera/","feedback":"🤖 Agent feedback for docs: (, )"}'`, + ].join(' '), '', '## Navigation', '', diff --git a/packages/@expo/prebuild-config/CHANGELOG.md b/packages/@expo/prebuild-config/CHANGELOG.md index d5ab72fbd00c29..5d3d95ad98fc2f 100644 --- a/packages/@expo/prebuild-config/CHANGELOG.md +++ b/packages/@expo/prebuild-config/CHANGELOG.md @@ -12,6 +12,7 @@ - Generated iOS projects now include a `SceneDelegate` and `UIApplicationSceneManifest` for the scene-based life cycle. ([#46734](https://github.com/expo/expo/pull/46734) by [@alanjhughes](https://github.com/alanjhughes)) - [iOS] Write the `RCTMetroPort` Info.plist key so bare dev builds resolve their own Metro port instead of defaulting to 8081. ([#48098](https://github.com/expo/expo/pull/48098) by [@alanjhughes](https://github.com/alanjhughes)) +- [iOS] Keep the Xcode project in sync when the app icon switches between a Liquid Glass `.icon` package and a PNG icon. ([#46070](https://github.com/expo/expo/pull/46070) by [@debugtheworldbot](https://github.com/debugtheworldbot)) ### 💡 Others diff --git a/packages/@expo/prebuild-config/src/plugins/icons/__tests__/withIosIcons-test.ts b/packages/@expo/prebuild-config/src/plugins/icons/__tests__/withIosIcons-test.ts index d75bc650c177be..fea38febde714b 100644 --- a/packages/@expo/prebuild-config/src/plugins/icons/__tests__/withIosIcons-test.ts +++ b/packages/@expo/prebuild-config/src/plugins/icons/__tests__/withIosIcons-test.ts @@ -1,12 +1,14 @@ import { WarningAggregator } from '@expo/config-plugins'; +import type { ExportedConfigWithProps, XcodeProject } from '@expo/config-plugins'; import type { ExpoConfig, IOSIcons } from '@expo/config-types'; import type * as fs from 'fs'; import { vol } from 'memfs'; import * as path from 'path'; +import xcode from 'xcode'; import rnFixture from '../../__tests__/fixtures/react-native-project'; import { getDirFromFS } from '../../__tests__/getDirFromFS'; -import { getIcons, setIconsAsync } from '../withIosIcons'; +import { getIcons, setIconsAsync, withIosIcons } from '../withIosIcons'; const fsReal = jest.requireActual('fs') as typeof fs; @@ -17,6 +19,10 @@ jest.mock('@expo/config-plugins', () => ({ WarningAggregator: { addWarningIOS: jest.fn(), }, + // Stubbed out for the whole file: the dangerous mod is skipped entirely (so `setIconsAsync` only + // runs where a test calls it directly), and the Xcode mod runs its action synchronously. + withDangerousMod: jest.fn((config: any) => config), + withXcodeProject: jest.fn((config: any, action: any) => action(config)), })); jest.mock('fs'); @@ -124,59 +130,152 @@ describe('iOS Icons', () => { }); }); -describe('e2e: iOS icons', () => { - const iconPath = path.resolve(__dirname, '../../__tests__/fixtures/icon.png'); - +describe(withIosIcons, () => { const projectRoot = '/app'; - beforeAll(async () => { - const icon = fsReal.readFileSync(iconPath); + const projectName = 'HelloWorld'; + const pbxprojPath = `ios/${projectName}.xcodeproj/project.pbxproj`; + const generatedAppIconName = 'AppIcon'; + // Seeded into the fixture, which already ships `generatedAppIconName`, so that "left alone" is + // distinguishable from "reset to the generated appiconset". + const staleAppIconName = 'oldIcon'; + const liquidGlassAppIconName = 'MyApp'; + + // Expected results of `getAppIconNames`. The fixture's app target has a Debug and a Release + // configuration, and the plugin writes to both. + const generatedAppIconNames = { + Debug: generatedAppIconName, + Release: generatedAppIconName, + }; + const staleAppIconNames = { Debug: staleAppIconName, Release: staleAppIconName }; + const liquidGlassAppIconNames = { + Debug: liquidGlassAppIconName, + Release: liquidGlassAppIconName, + }; + + afterEach(() => { + vol.reset(); + }); - vol.fromJSON(rnFixture, projectRoot); + function parseProjectWithStaleAppIconName(): XcodeProject { + const pbxproj = rnFixture[pbxprojPath].replace( + new RegExp(`ASSETCATALOG_COMPILER_APPICON_NAME = ${generatedAppIconName};`, 'g'), + `ASSETCATALOG_COMPILER_APPICON_NAME = ${staleAppIconName};` + ); - vol.mkdirpSync('/app/assets'); - vol.writeFileSync('/app/assets/icon.png', icon); - }); + vol.fromJSON({ [pbxprojPath]: pbxproj }, projectRoot); - afterAll(() => { - vol.reset(); - }); + const project = xcode.project(path.join(projectRoot, pbxprojPath)); + project.parseSync(); + return project; + } - it('writes all the image files expected', async () => { - await setIconsAsync( - { - slug: 'HelloWorld', - version: '1', - name: 'HelloWorld', - platforms: ['ios', 'android'], - // must use full path for mock fs - icon: '/app/assets/icon.png', - }, - projectRoot + function runPlugin( + project: XcodeProject, + config: Pick, + modRequest: { projectName?: string } = { projectName } + ) { + withIosIcons({ + slug: projectName, + version: '1', + name: projectName, + platforms: ['ios'], + ...config, + modResults: project, + modRequest, + } as ExportedConfigWithProps); + } + + // Keyed by build configuration name, so asserting on the whole map catches a partial update. + function getAppIconNames(project: XcodeProject): Record { + const configurations = Object.entries(project.pbxXCBuildConfigurationSection()).filter( + ([key]) => !key.endsWith('_comment') ); - const after = getDirFromFS(vol.toJSON(), projectRoot); - const icons = Object.keys(after).filter((value) => - value.startsWith('ios/HelloWorld/Images.xcassets/AppIcon.appiconset/App-Icon') + return Object.fromEntries( + configurations + .map(([, configuration]) => [ + (configuration as any).name, + (configuration as any).buildSettings?.ASSETCATALOG_COMPILER_APPICON_NAME, + ]) + .filter(([, appIconName]) => appIconName !== undefined) ); + } - expect(WarningAggregator.addWarningIOS).toHaveBeenCalledTimes(0); + it.each<[string, Pick]>([ + ['a PNG icon in `ios.icon`', { ios: { icon: './assets/icon.png' } }], + [ + 'appearance-aware icons in `ios.icon`', + { ios: { icon: { light: './assets/light.png', dark: './assets/dark.png' } } }, + ], + ['a PNG icon in the root `icon`', { icon: './assets/icon.png' }], + ])('resets the app icon name to the generated appiconset for %s', (_, config) => { + const project = parseProjectWithStaleAppIconName(); - expect(icons.length).toBe(1); + runPlugin(project, config); - // Test the Contents.json file - const contents = JSON.parse( - after['ios/HelloWorld/Images.xcassets/AppIcon.appiconset/Contents.json']! + expect(getAppIconNames(project)).toEqual(generatedAppIconNames); + }); + + it('points the app icon name at a `.icon` package and adds it to the project', () => { + const project = parseProjectWithStaleAppIconName(); + + runPlugin(project, { ios: { icon: `assets/${liquidGlassAppIconName}.icon` } }); + + const output = project.writeSync(); + expect(getAppIconNames(project)).toEqual(liquidGlassAppIconNames); + expect(output).toContain(`/* ${liquidGlassAppIconName}.icon in Resources */`); + expect(output).toContain(`path = "${projectName}/${liquidGlassAppIconName}.icon"`); + }); + + it('removes stale `.icon` resource references when switching back to a PNG icon', () => { + const project = parseProjectWithStaleAppIconName(); + + // A prebuild that configured a liquid glass icon... + runPlugin(project, { ios: { icon: `assets/${liquidGlassAppIconName}.icon` } }); + expect(getAppIconNames(project)).toEqual(liquidGlassAppIconNames); + expect(project.writeSync()).toContain(`${liquidGlassAppIconName}.icon`); + + // ...followed by one where the user switched back to a PNG icon. + runPlugin(project, { ios: { icon: './assets/icon.png' } }); + + expect(getAppIconNames(project)).toEqual(generatedAppIconNames); + // `setIconsAsync` no longer copies the `.icon` package into the project, so leaving its build + // file and file references behind would point the build at a file that isn't there. + expect(project.writeSync()).not.toContain(`${liquidGlassAppIconName}.icon`); + }); + + it('leaves the app icon name alone when no icon is configured', () => { + const project = parseProjectWithStaleAppIconName(); + + runPlugin(project, {}); + + expect(getAppIconNames(project)).toEqual(staleAppIconNames); + }); + + it('leaves the app icon name alone when the project name is unknown', () => { + const project = parseProjectWithStaleAppIconName(); + + runPlugin( + project, + { ios: { icon: `assets/${liquidGlassAppIconName}.icon` } }, + { projectName: undefined } ); - expect(contents.images).toMatchSnapshot(); - // Ensure all icons are assigned as expected. - expect(contents.images.length).toBe(1); + expect(getAppIconNames(project)).toEqual(staleAppIconNames); }); }); -describe('e2e: iOS icons with fallback image', () => { + +describe('e2e: iOS icons', () => { + const iconPath = path.resolve(__dirname, '../../__tests__/fixtures/icon.png'); + const projectRoot = '/app'; beforeAll(async () => { + const icon = fsReal.readFileSync(iconPath); + vol.fromJSON(rnFixture, projectRoot); + + vol.mkdirpSync('/app/assets'); + vol.writeFileSync('/app/assets/icon.png', icon); }); afterAll(() => { @@ -190,7 +289,8 @@ describe('e2e: iOS icons with fallback image', () => { version: '1', name: 'HelloWorld', platforms: ['ios', 'android'], - // No icon should be set + // must use full path for mock fs + icon: '/app/assets/icon.png', }, projectRoot ); @@ -200,6 +300,8 @@ describe('e2e: iOS icons with fallback image', () => { value.startsWith('ios/HelloWorld/Images.xcassets/AppIcon.appiconset/App-Icon') ); + expect(WarningAggregator.addWarningIOS).toHaveBeenCalledTimes(0); + expect(icons.length).toBe(1); // Test the Contents.json file @@ -214,9 +316,16 @@ describe('e2e: iOS icons with fallback image', () => { }); describe('e2e: iOS liquid glass icons', () => { + const iconPath = path.resolve(__dirname, '../../__tests__/fixtures/icon.png'); const projectRoot = '/app'; + const generatedIconPath = + 'ios/HelloWorld/Images.xcassets/AppIcon.appiconset/App-Icon-1024x1024@1x.png'; + const generatedIconContentsPath = + 'ios/HelloWorld/Images.xcassets/AppIcon.appiconset/Contents.json'; + + beforeEach(() => { + const icon = fsReal.readFileSync(iconPath); - beforeAll(async () => { vol.fromJSON( { ...rnFixture, @@ -228,68 +337,116 @@ describe('e2e: iOS liquid glass icons', () => { }, projectRoot ); + + vol.mkdirpSync('/app/assets'); + vol.writeFileSync('/app/assets/icon.png', icon); }); - afterAll(() => { + afterEach(() => { vol.reset(); }); - it('detects .icon directories correctly', () => { - const config = { - ios: { icon: 'assets/MyApp.icon' }, - }; - - const icon = getIcons(config); - expect(icon).toBe('assets/MyApp.icon'); + async function generatePngAppIconAsync() { + await setIconsAsync( + { + slug: 'HelloWorld', + version: '1', + name: 'HelloWorld', + platforms: ['ios', 'android'], + icon: '/app/assets/icon.png', + }, + projectRoot + ); - if (typeof icon === 'string') { - expect(path.extname(icon)).toBe('.icon'); - } - }); + const after = getDirFromFS(vol.toJSON(), projectRoot); + expect(after[generatedIconPath]).toBeDefined(); + expect(after[generatedIconContentsPath]).toBeDefined(); + } - it('processes .icon directories without warnings', async () => { - const config: ExpoConfig = { - slug: 'HelloWorld', - version: '1', - name: 'HelloWorld', - platforms: ['ios', 'android'], - ios: { - icon: 'assets/MyApp.icon', + function setLiquidGlassIconAsync(icon: string) { + return setIconsAsync( + { + slug: 'HelloWorld', + version: '1', + name: 'HelloWorld', + platforms: ['ios', 'android'], + ios: { icon }, }, - }; - - const icon = getIcons(config); - expect(icon).toBe('assets/MyApp.icon'); - if (typeof icon === 'string') { - expect(path.extname(icon)).toBe('.icon'); - } + projectRoot + ); + } - (WarningAggregator.addWarningIOS as jest.Mock).mockClear(); + it('replaces the generated PNG app icon with the .icon package', async () => { + await generatePngAppIconAsync(); - await setIconsAsync(config, projectRoot); + await setLiquidGlassIconAsync('assets/MyApp.icon'); + // The appiconset is what the PNG path generates, so it has to go once a `.icon` package owns + // the app icon — otherwise the project carries two competing icon sources. + const after = getDirFromFS(vol.toJSON(), projectRoot); + expect(after[generatedIconPath]).toBeUndefined(); + expect(after[generatedIconContentsPath]).toBeUndefined(); + expect(after['ios/HelloWorld/MyApp.icon/icon.json']).toBe( + JSON.stringify({ + version: 1, + format: 'liquid-glass-icon', + }) + ); expect(WarningAggregator.addWarningIOS).toHaveBeenCalledTimes(0); }); - it('warns when .icon file does not exist', async () => { - (WarningAggregator.addWarningIOS as jest.Mock).mockClear(); + it('keeps the generated PNG app icon when the .icon package is missing', async () => { + await generatePngAppIconAsync(); + + await setLiquidGlassIconAsync('assets/DoesNotExist.icon'); + + // Nothing was copied in, so removing the generated icons would leave no app icon at all. + const after = getDirFromFS(vol.toJSON(), projectRoot); + expect(after[generatedIconPath]).toBeDefined(); + expect(after[generatedIconContentsPath]).toBeDefined(); + expect(WarningAggregator.addWarningIOS).toHaveBeenCalledWith( + 'icon', + 'Liquid glass icon file not found at path: assets/DoesNotExist.icon' + ); + }); +}); + +describe('e2e: iOS icons with fallback image', () => { + const projectRoot = '/app'; + beforeAll(async () => { + vol.fromJSON(rnFixture, projectRoot); + }); + afterAll(() => { + vol.reset(); + }); + + it('writes all the image files expected', async () => { await setIconsAsync( { slug: 'HelloWorld', version: '1', name: 'HelloWorld', platforms: ['ios', 'android'], - ios: { - icon: 'assets/DoesNotExist.icon', - }, + // No icon should be set }, projectRoot ); - expect(WarningAggregator.addWarningIOS).toHaveBeenCalledWith( - 'icon', - 'Liquid glass icon file not found at path: assets/DoesNotExist.icon' + const after = getDirFromFS(vol.toJSON(), projectRoot); + const icons = Object.keys(after).filter((value) => + value.startsWith('ios/HelloWorld/Images.xcassets/AppIcon.appiconset/App-Icon') + ); + + expect(icons.length).toBe(1); + + // Test the Contents.json file + const contents = JSON.parse( + after['ios/HelloWorld/Images.xcassets/AppIcon.appiconset/Contents.json']! ); + expect(contents.images).toMatchSnapshot(); + + // Ensure all icons are assigned as expected. + expect(contents.images.length).toBe(1); }); }); diff --git a/packages/@expo/prebuild-config/src/plugins/icons/withIosIcons.ts b/packages/@expo/prebuild-config/src/plugins/icons/withIosIcons.ts index e185cb173f7428..9c7d5245f9fce2 100644 --- a/packages/@expo/prebuild-config/src/plugins/icons/withIosIcons.ts +++ b/packages/@expo/prebuild-config/src/plugins/icons/withIosIcons.ts @@ -14,10 +14,11 @@ import path from 'path'; import type { ContentsJson, ContentsJsonImage } from './AssetContents'; import { writeContentsJsonAsync } from './AssetContents'; -const { getProjectName } = IOSConfig.XcodeUtils; +const { getProjectName, unquote } = IOSConfig.XcodeUtils; const IMAGE_CACHE_NAME = 'icons'; const IMAGESET_PATH = 'Images.xcassets/AppIcon.appiconset'; +const DEFAULT_APPICON_NAME = 'AppIcon'; export const withIosIcons: ConfigPlugin = (config) => { config = withDangerousMod(config, [ @@ -32,10 +33,14 @@ export const withIosIcons: ConfigPlugin = (config) => { const icon = getIcons(config); const projectName = config.modRequest.projectName; - if (icon && typeof icon === 'string' && path.extname(icon) === '.icon' && projectName) { + if (projectName && icon && typeof icon === 'string' && path.extname(icon) === '.icon') { const iconName = path.basename(icon, '.icon'); setIconName(config.modResults, projectName, iconName); addIconFileToProject(config.modResults, projectName, iconName); + } else if (projectName && icon) { + const previousIconNames = getIconNames(config.modResults, projectName); + setIconName(config.modResults, projectName, DEFAULT_APPICON_NAME); + removeIconFilesFromProject(config.modResults, projectName, previousIconNames); } return config; }); @@ -96,7 +101,10 @@ export async function setIconsAsync(config: ExpoConfig, projectRoot: string) { const iosNamedProjectRoot = getIosNamedProjectPath(projectRoot); if (typeof icon === 'string' && path.extname(icon) === '.icon') { - return await addLiquidGlassIcon(icon, projectRoot, iosNamedProjectRoot); + if (await addLiquidGlassIcon(icon, projectRoot, iosNamedProjectRoot)) { + await removeGeneratedIconImagesAsync(iosNamedProjectRoot); + } + return; } // Ensure the Images.xcassets/AppIcon.appiconset path exists @@ -233,7 +241,7 @@ async function addLiquidGlassIcon( iconPath: string, projectRoot: string, iosNamedProjectRoot: string -): Promise { +): Promise { const iconName = path.basename(iconPath, '.icon'); const sourceIconPath = path.join(projectRoot, iconPath); const targetIconPath = path.join(iosNamedProjectRoot, `${iconName}.icon`); @@ -243,10 +251,18 @@ async function addLiquidGlassIcon( 'icon', `Liquid glass icon file not found at path: ${iconPath}` ); - return; + return false; } await fs.promises.cp(sourceIconPath, targetIconPath, { recursive: true }); + return true; +} + +async function removeGeneratedIconImagesAsync(iosNamedProjectRoot: string): Promise { + await fs.promises.rm(path.join(iosNamedProjectRoot, IMAGESET_PATH), { + force: true, + recursive: true, + }); } /** @@ -280,3 +296,50 @@ function addIconFileToProject(project: any, projectName: string, iconName: strin verbose: true, }); } + +function getIconNames(project: any, projectName: string): string[] { + const [, target] = findNativeTargetByName(project, projectName); + const configurations = IOSConfig.XcodeUtils.getBuildConfigurationsForListId( + project, + target.buildConfigurationList + ); + + return Array.from( + new Set( + configurations + .map(([, config]) => (config as any)?.buildSettings?.ASSETCATALOG_COMPILER_APPICON_NAME) + .filter((iconName): iconName is string => typeof iconName === 'string') + .map(unquote) + ) + ); +} + +function removeIconFilesFromProject(project: any, projectName: string, iconNames: string[]): void { + const [targetUuid] = findNativeTargetByName(project, projectName); + const groupKey = project.findPBXGroupKey({ name: projectName }); + + for (const iconName of iconNames) { + removeIconFileFromProject(project, `${projectName}/${iconName}.icon`, targetUuid, groupKey); + } +} + +function removeIconFileFromProject( + project: any, + iconPath: string, + targetUuid: string, + groupKey?: string +): void { + const file = { + basename: path.basename(iconPath), + group: 'Resources', + path: iconPath, + target: targetUuid, + }; + + project.removeFromPbxBuildFileSection(file); + project.removeFromPbxFileReferenceSection(file); + if (groupKey) { + project.removeFromPbxGroup(file, groupKey); + } + project.removeFromPbxResourcesBuildPhase(file); +}