feat: add AppScope support, switch to commonjs#13
Open
Groupguanfang wants to merge 3 commits intomainfrom
Open
feat: add AppScope support, switch to commonjs#13Groupguanfang wants to merge 3 commits intomainfrom
Groupguanfang wants to merge 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds AppScope support to the project detector library and migrates from ESM (ES modules) to CommonJS module format.
Key Changes:
- Introduces a new
AppScopestruct and API for handling HarmonyOS app-level configuration - Refactors reference handling throughout the Rust codebase to use
Rc<Reference<T>>for better memory management - Converts module system from ESM to CommonJS (
type: "module"removed, exports changed)
Reviewed changes
Copilot reviewed 29 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/rust/app_scope.rs |
New AppScope implementation for app.json5 configuration handling |
src/rust/resource.rs |
Added ResourceType enum and AppScope integration with Resource |
src/rust/resource_directory.rs |
Improved reference cloning with Rc wrapper |
src/rust/project.rs |
Refactored to use Rc for ProjectDetector references |
src/rust/product.rs |
Updated Module reference handling with Rc |
src/rust/module.rs |
Improved Project reference cloning patterns |
src/rust/utils/qualifier/utils_impl.rs |
Converted block comments to doc comments with TypeScript type hints |
src/rust/references/element_json_file_reference.rs |
Enhanced error handling for parser operations |
src/node/app-scope.ts |
TypeScript wrapper for AppScope with signal-based reactivity |
index.js |
Converted from ESM to CommonJS exports |
package.json |
Removed "type": "module" and updated build command |
test/node.spec.ts |
Added comprehensive tests for AppScope functionality |
| Mock data files | Added AppScope test fixtures for both harmony projects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (project.getBuildProfileUri().isEqual(uri) || project.getUri().isEqual(uri)) { | ||
| appScope(RustAppScope.from(project.getUnderlyingProject())) | ||
| } | ||
| else if (uri.fsPath.endsWith('app.json5')) { |
There was a problem hiding this comment.
The comparison uri.fsPath.endsWith('app.json5') is fragile and could match unintended files. Consider using a more robust check that ensures the file is specifically in the AppScope directory, such as checking the full path or using uri.isEqual() with the expected app.json5 URI.
Suggested change
| else if (uri.fsPath.endsWith('app.json5')) { | |
| else if (uri.isEqual(project.getAppJson5Uri())) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.