Watch all imported files on app dev #6078
Conversation
gonzaloriestra
added
the
stale-exempt
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3256 tests passing in 1367 suites. Report generated by 🧪jest coverage report action from 2641765 |
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
gonzaloriestra
force-pushed
the
watch-imports
branch
from
e129013 to
2641765
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationspackages/cli-kit/dist/public/node/import-extractor.d.ts/**
* Extracts import paths from a source file.
* Supports JavaScript, TypeScript, and Rust files.
*
* @param filePath - Path to the file to analyze.
* @returns Array of absolute paths to imported files.
*/
export declare function extractImportPaths(filePath: string): string[];
Existing type declarationsWe found no diffs with existing type declarations |
| private getExtensionEntryFiles(extension: ExtensionInstance): string[] { | ||
| const entryFiles: string[] = [] | ||
|
|
||
| // First, check if we have an explicit entrySourceFilePath | ||
| if (extension.entrySourceFilePath) { | ||
| entryFiles.push(extension.entrySourceFilePath) | ||
| return entryFiles | ||
| } | ||
|
|
||
| // For UI extensions, check targeting/extension_points configuration | ||
| const config = extension.configuration | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const targetingConfig = (config as any).targeting || (config as any).extension_points | ||
| if (targetingConfig && Array.isArray(targetingConfig)) { | ||
| for (const target of targetingConfig) { | ||
| if (target.module) { | ||
| const modulePath = joinPath(extension.directory, target.module) | ||
| if (fileExistsSync(modulePath)) { | ||
| entryFiles.push(modulePath) | ||
| } |
There was a problem hiding this comment.
This function should probably be in extension-instance
Also, I think the implementation is wrong. For ui_extensions we generate custom in-memory entry files in getBundleExtensionStdinContent, we need to use that to watch for changes.
This already returns a string with format import ..., so it can be passed directly to the parser. Maybe with a new function extractImportPathFromFileContent(content: string)
| /** | ||
| * Performs the actual rescan of extension imports | ||
| */ | ||
| private async doRescanExtensionImports(extensionPath: string): Promise<void> { |
There was a problem hiding this comment.
Why have both scanExtensionsForImports and doRescanExtensionImports? they seem to be doing almost the exact thing.
We can have a shared scanImportsForExtension(extension...) and then call it either for ALL or just for the one that was just updated.
WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/11518
When running app dev, we need to watch for changes in imported files that live outside of the extension folders.
WHAT is this pull request doing?
How to test your changes?
Measuring impact
How do we know this change was effective? Please choose one:
Checklist