-
Notifications
You must be signed in to change notification settings - Fork 225
Watch all imported files on app dev #6078
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
Conversation
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3280 tests passing in 1352 suites. Report generated by 🧪jest coverage report action from 2ddec70 |
|
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. |
e129013 to
2641765
Compare
63ac258 to
e974896
Compare
|
@isaacroldan Thanks for the suggestions! I've made some improvements:
|
27509ae to
9cb4744
Compare
|
/snapit |
|
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/[email protected]Caution After installing, validate the version by running just |
9cb4744 to
470f157
Compare
packages/app/src/cli/models/extensions/extension-instance.test.ts
Outdated
Show resolved
Hide resolved
packages/app/src/cli/services/dev/app-events/file-watcher.test.ts
Outdated
Show resolved
Hide resolved
09769a0 to
2ddec70
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[];
/**
* Recursively extracts import paths from a source file and all its dependencies.
* Supports JavaScript, TypeScript, and Rust files.
* Handles circular dependencies by tracking visited files.
*
* @param filePath - Path to the file to analyze.
* @param visited - Set of already visited files to prevent infinite recursion.
* @returns Array of absolute paths to the provided file and all imported files there (including nested imports).
* @throws If an unexpected error occurs while processing files (not including ENOENT file not found errors).
*/
export declare function extractImportPathsRecursively(filePath: string, visited?: Set<string>): string[];
/**
* Extracts import paths from a JavaScript content.
*
* @param content - The content to extract imports from.
* @param filePath - The path to the file to extract imports from.
* @returns Array of absolute paths to imported files.
*/
export declare function extractJSImports(content: string, filePath: string): string[];
Existing type declarationspackages/cli-kit/dist/public/node/fs.d.ts@@ -155,6 +155,13 @@ export declare function mkTmpDir(): Promise<string>;
* @returns True if the path is a directory, false otherwise.
*/
export declare function isDirectory(path: string): Promise<boolean>;
+/**
+ * Check whether a path is a directory.
+ *
+ * @param path - Path to check.
+ * @returns True if the path is a directory, false otherwise.
+ */
+export declare function isDirectorySync(path: string): boolean;
/**
* Get the size of a file.
*
@@ -273,6 +280,14 @@ export declare function generateRandomNameForSubdirectory(options: GenerateRando
* @returns A promise that resolves to an array of pathnames that match the given pattern.
*/
export declare function glob(pattern: Pattern | Pattern[], options?: GlobOptions): Promise<string[]>;
+/**
+ * Synchronously traverse the file system and return pathnames that match the given pattern.
+ *
+ * @param pattern - A glob pattern or an array of glob patterns.
+ * @param options - Options for the glob.
+ * @returns An array of pathnames that match the given pattern.
+ */
+export declare function globSync(pattern: Pattern | Pattern[], options?: GlobOptions): string[];
/**
* Convert a path to a File URL.
*
|
isaacroldan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on this 👏
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?
watchis defined under[extensions.build], only those files are watchedHow to test your changes?
npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/[email protected]Instructions to add a UI extension with assets (should_render): https://gist.github.com/elanalynn/bff3bda620cb5d0284d0cf892e713e90
Measuring impact
How do we know this change was effective? Please choose one:
Checklist