feat: support define page #145
Closed
Conversation
This file contains 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
WalkthroughThe update entails enhancements in ESLint integration, Vue.js project page management, and development workflow optimizations. It introduces new features for defining and parsing pages, improves ESLint settings in VS Code, and streamlines interactions with Vue Single File Components (SFCs). Utility functions are introduced, and the playground showcases enhanced capabilities, emphasizing code quality and developer efficiency. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
package.jsonis excluded by:!**/*.jsonpackages/core/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (10)
- .vscode/settings.json (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (8 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/i18n.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/playground/src/uni-pages.d.ts
Additional comments: 18
packages/playground/src/pages/i18n.vue (1)
- 5-11: The usage of
definePagewithin the<script setup>block correctly implements the new feature. This change aligns with the PR's objective to support thedefinePagemacro, allowing for a more structured and declarative way to define page options. Ensure that the%app.name%placeholder is properly resolved in the context of your application.packages/core/src/constant.ts (1)
- 8-8: The addition of the
DEFINE_PAGEconstant is correct and aligns with the PR's objective to support thedefinePagemacro. This constant will likely be used throughout the framework to reference the macro by name, ensuring consistency.packages/core/client.d.ts (1)
- 9-11: The declaration of the
definePagefunction within theglobalThismodule is correctly implemented, allowing it to be recognized globally. This aligns with the PR's objective to introduce thedefinePagemacro. Ensure that theDefinePageOptionstype accurately reflects the options that can be passed todefinePage..vscode/settings.json (1)
- 2-42: The ESLint configuration adjustments, including enabling flat config support, disabling the default formatter in favor of ESLint, configuring editor settings for ESLint auto-fix, customizing ESLint rules severity, and enabling ESLint validation for various languages, are correctly implemented. These changes enhance the development experience by ensuring code quality and consistency. Ensure that these settings align with your project's coding standards and ESLint rules.
packages/core/src/utils.ts (2)
- 2-2: Changing the import statement for
ModuleNodeandViteDevServerfrom 'vite' to use type syntax is a good practice, as it clarifies that these imports are only used for type information and not included in the JavaScript output.- 54-56: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [36-60]
The removal of unused functions (
getRouteSfcBlockanduseCachedPages) and their internal logic is a positive change, as it helps to keep the codebase clean and maintainable. Ensure that these functions are indeed not used anywhere in the project to avoid breaking changes.packages/core/src/types.ts (2)
- 15-16: The addition of the
RouteBlockLangtype with specific string literals is a good practice, as it restricts the values to a predefined set, enhancing type safety and readability.- 146-152: The introduction of the
DefinePageOptionsinterface with a deprecatedpathproperty is correctly implemented. However, ensure that the deprecation of thepathproperty is clearly communicated to developers, possibly through documentation, to avoid confusion.packages/core/src/index.ts (1)
- 83-115: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-149]
The modifications in
index.ts, including importing necessary functions from@vue/compiler-sfc, adding new constants, and adjusting thetransformfunction to handle parsing of Vue single-file components (SFC), finding macros, and route blocks, are correctly implemented. These changes align with the PR's objective to support thedefinePagemacro and enhance code transformation and parsing capabilities. Ensure that thetransformfunction's logic correctly handles all edge cases and that the error messages are clear and informative for developers.packages/core/src/page.ts (2)
- 16-60: The introduction of the
Pageclass, which centralizes the reading and parsing of page options from various file formats, is correctly implemented. This class enhances the maintainability and modularity of the codebase by encapsulating page-related logic. Ensure that the error handling in thereadOptionsmethod is robust and provides clear error messages to developers.- 63-149: The functions
readPageOptionsFromFile,readPageOptionsFromMacro, andreadPageOptionsFromBlockare well-implemented, providing a structured approach to parsing page options based on the file content and the specified route block language. Ensure that the parsing logic correctly handles all supported formats and that the error messages are informative.packages/core/src/context.ts (7)
- 22-22: The import of the
Pageclass aligns with the PR's objective to support thedefinePagemacro by introducing a centralized way to manage page metadata and options. This change is crucial for the refactoring efforts to work withPageinstances.- 44-45: The introduction of
pagesandsubPagesas class properties is a significant change. It shifts the data structure from using paths to usingPageinstances, which likely enhances the flexibility and capabilities of page management within the framework. However, ensure that all references to these properties throughout the codebase have been updated accordingly.Verification successful
The examination of the context around
subPagesPathindeclaration.tsindicates that it is used to generate paths for subpages, derived fromctx.subPageMetaData. This usage does not directly reference the newsubPagesproperty but seems to align with the intent of managing page paths within the framework. There's no evidence from the provided context that references to the old structure were not updated correctly. Therefore, based on the information available, it appears that the concern raised in the original review comment has been addressed as far as can be determined.* 83-83: The refactoring of the `scanPages` method to work with `Page` instances instead of paths is a key part of integrating the `definePage` macro. This change should streamline the page initialization process. However, it's important to ensure that the `Page` class's constructor and methods are designed to handle all necessary page-related functionalities efficiently.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all references to `pages` and `subPages` are correctly updated. rg --type ts "pagesPath|subPagesPath"Length of output: 301
Script:
#!/bin/bash # Examine the context around the usage of `subPagesPath` in declaration.ts to understand its relation to the new `subPages` property. rg --type ts -C 5 "subPagesPath" packages/core/src/declaration.tsLength of output: 714
Verification successful
The Page class's constructor and methods, including
getOptions,hasChanged, andreadOptions, are designed to efficiently and comprehensively handle page-related functionalities. The constructor initializes the context and file path, while the methods manage the reading, caching, and change detection of page options. This setup should support the integration of thedefinePagemacro effectively by streamlining the page initialization process.* 87-97: The `scanSubPages` method has been updated to also work with `Page` instances, similar to the `scanPages` method. This consistency in handling both pages and sub-pages using the same data structure is beneficial. Ensure that the logic for handling sub-pages is thoroughly tested, especially in scenarios involving nested sub-pages or complex directory structures. * 181-182: The `parsePages` method's update to work with `Page` instances and potentially custom page metadata is a critical part of the new page handling logic. It's important to ensure that this method accurately merges generated and custom page metadata, especially when overrides are provided. Consider adding tests to verify the correctness of the merged metadata under various scenarios. * 250-262: The `getPageByPath` method has been updated to retrieve `Page` instances by their absolute path. This method is crucial for efficiently finding pages, especially during the update process. Ensure that this method is optimized for performance, considering the potential size of the `pages` and `subPages` collections. * 266-267: The logic within `updatePagesJSON` to skip updates if a page has not changed is a good optimization. However, ensure that the `hasChanged` method of the `Page` class is robust and accurately detects changes to avoid unnecessary updates or missing necessary ones.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the efficiency and completeness of the Page class's constructor and methods. ast-grep --lang typescript --pattern $'class Page { $$$ constructor($_, $_) { $$$ } $$$ }'Length of output: 2632
Verification successful
The
hasChangedmethod in thePageclass is designed to be robust, leveraging thereadOptionsmethod to accurately detect changes by comparing the serialized current options with a previously stored version. This approach, combined with exception handling in thereadOptionsmethod, ensures a reliable mechanism for detecting changes in page options. Therefore, the optimization to skip updates if a page has not changed, as implemented, appears to be well-founded.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the robustness of the Page class's hasChanged method. ast-grep --lang typescript --pattern $'class Page { $$$ hasChanged() { $$$ } $$$ }'Length of output: 2625
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- README.md (1 hunks)
- packages/core/src/page.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/core/src/page.ts
Additional comments: 2
README.md (2)
- 57-73: The documentation for the
definePagemacro is clear and provides useful examples. However, it might be beneficial for readers if a brief explanation of the benefits or specific use cases of usingdefinePageover traditional methods is included.- 75-75: The documentation for using SFC custom blocks for route data is comprehensive and informative. It might be helpful to include a note about any potential limitations or considerations when opting for this method to define routing data.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (7)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/playground/src/uni-pages.d.ts
Additional comments: 5
packages/core/src/context.ts (5)
- 21-21: The addition of the
Pageimport aligns with the PR's objectives and supports the new page management approach.- 43-45: The introduction of
pagesandsubPagesas class properties is a significant and positive change, facilitating the new approach to page management using thePageclass.- 79-82: Refactoring the page initialization and scanning logic to work with
Pageinstances instead ofPagePathis a significant improvement, enhancing the module's functionality and maintainability.- 180-180: The method
parsePageshas been updated to work withPageinstances, which is a crucial change for integrating thedefinePagemacro. This update enhances the flexibility and accuracy of page parsing.- 249-261: The method
getPageByPathhas been updated to retrieve pages by path usingPageinstances. This change improves the accuracy and efficiency of page retrieval.
Comment on lines
1
to
14
| <script lang="ts" setup> | ||
| definePage({ | ||
| style:{ | ||
| navigationBarTitleText: 'hello world' | ||
| }, | ||
| middlewares: [ | ||
| 'auth' | ||
| ] | ||
| }) | ||
| </script> | ||
|
|
||
| <template> | ||
| <div>test</div> | ||
| </template> |
There was a problem hiding this comment.
The implementation of the definePage macro in a Vue component using an object configuration looks good. Ensure that there is sufficient documentation and tests covering the definePage macro to facilitate its adoption and ensure its reliability.
Would you like assistance in creating documentation or tests for the definePage macro?
Comment on lines
1
to
21
| <script lang="ts" setup> | ||
| definePage(()=>{ | ||
|
|
||
| const hello = 'hello' | ||
|
|
||
| const world = 'world' | ||
|
|
||
| return { | ||
| style:{ | ||
| navigationBarTitleText: [hello, world].join(' ') | ||
| }, | ||
| middlewares: [ | ||
| 'auth' | ||
| ] | ||
| } | ||
| }) | ||
| </script> | ||
|
|
||
| <template> | ||
| <div>test</div> | ||
| </template> |
There was a problem hiding this comment.
The implementation of the definePage macro in a Vue component using a function configuration looks good. Ensure that there is sufficient documentation and tests covering the definePage macro to facilitate its adoption and ensure its reliability.
Would you like assistance in creating documentation or tests for the definePage macro?
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (4)
- README.md (1 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- README.md
- packages/core/src/page.ts
- packages/playground/src/uni-pages.d.ts
Comment on lines
2
to
17
| definePage(async ()=>{ | ||
|
|
||
| const randomTxt = await new Promise(resolve=> { | ||
| const txt = Math.random().toString(36).slice(-8) | ||
| resolve(txt) | ||
| }); | ||
|
|
||
| return { | ||
| style:{ | ||
| navigationBarTitleText: randomTxt | ||
| }, | ||
| middlewares: [ | ||
| 'auth' | ||
| ] | ||
| } | ||
| }) |
There was a problem hiding this comment.
The usage of definePage with an async function is a powerful feature, allowing for dynamic page configuration. However, consider the following points for improvement:
- Error Handling: The async function lacks error handling. Wrapping the promise in a try-catch block would ensure that any errors during the promise resolution are caught and handled appropriately.
- Code Clarity: While the current implementation is concise, adding a brief comment explaining the purpose of generating a random string for
navigationBarTitleTextcould improve code readability and maintainability. - Middleware Documentation: If
middlewareslike 'auth' have specific requirements or behaviors, documenting them near their usage can help future developers understand their purpose and how to use them effectively.
definePage(async ()=>{
+ // Generate a random string to dynamically set the navigation bar title
+ try {
const randomTxt = await new Promise(resolve=> {
const txt = Math.random().toString(36).slice(-8)
resolve(txt)
});
+ } catch (error) {
+ console.error("Failed to generate random text for navigation bar title", error);
+ // Handle error appropriately
+ }
return {
style:{
navigationBarTitleText: randomTxt
},
+ // 'auth' middleware ensures that the user is authenticated before accessing this page
middlewares: [
'auth'
]
}
})Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| definePage(async ()=>{ | |
| const randomTxt = await new Promise(resolve=> { | |
| const txt = Math.random().toString(36).slice(-8) | |
| resolve(txt) | |
| }); | |
| return { | |
| style:{ | |
| navigationBarTitleText: randomTxt | |
| }, | |
| middlewares: [ | |
| 'auth' | |
| ] | |
| } | |
| }) | |
| definePage(async ()=>{ | |
| // Generate a random string to dynamically set the navigation bar title | |
| try { | |
| const randomTxt = await new Promise(resolve=> { | |
| const txt = Math.random().toString(36).slice(-8) | |
| resolve(txt) | |
| }); | |
| } catch (error) { | |
| console.error("Failed to generate random text for navigation bar title", error); | |
| // Handle error appropriately | |
| } | |
| return { | |
| style:{ | |
| navigationBarTitleText: randomTxt | |
| }, | |
| // 'auth' middleware ensures that the user is authenticated before accessing this page | |
| middlewares: [ | |
| 'auth' | |
| ] | |
| } | |
| }) |
edwinhuish
force-pushed
the
feat/support-definePage
branch
from
0aaac11 to
0d0b0b1
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
package.jsonis excluded by:!**/*.jsonpackages/core/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (13)
- README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/i18n.vue (1 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (11)
- README.md
- packages/core/client.d.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/playground/src/pages/i18n.vue
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/uni-pages.d.ts
Additional comments: 5
packages/core/src/utils.ts (1)
- 2-2: The change to use type imports for
ModuleNodeandViteDevServeris a good practice in TypeScript, as it clarifies that these imports are only used for type information. This can help with build optimization and clarity in the codebase.packages/core/src/context.ts (4)
- 21-21: The changes in import statements, including the removal of old page management imports and the addition of the
Pageimport, reflect a significant shift towards a more object-oriented approach in managing pages. This aligns well with the introduction of thedefinePagemacro and should enhance modularity and maintainability.- 43-44: The modifications to the
PageContextclass, including the use ofPageinstances for managing pages and subpages, represent a significant improvement in the architecture. This approach allows for better encapsulation of page-related logic and data, enhancing the flexibility and maintainability of the codebase.- 180-180: The updates to methods for parsing and managing pages, including changes to work with
Pageinstances, significantly improve the page management workflow. These changes enhance the clarity and efficiency of page parsing and retrieval, aligning well with the new object-oriented approach.- 82-82: The refactoring of page initialization and scanning logic to accommodate the new
Pageclass represents a significant improvement in the codebase. This change simplifies the page management process, making the code cleaner, more maintainable, and easier to extend.
edwinhuish
force-pushed
the
feat/support-definePage
branch
from
0d0b0b1 to
2ec3799
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
packages/core/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (12)
- README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (11)
- README.md
- packages/core/client.d.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/core/src/utils.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/uni-pages.d.ts
Additional comments: 7
packages/core/src/context.ts (7)
- 21-21: The import of the
Pageclass is correctly added to support the newdefinePagemacro feature. Ensure that thePageclass is implemented as expected and integrates well with the existing codebase.- 43-44: The declarations of
pagesandsubPagesas class properties are appropriate for managing instances ofPage. This change aligns with the objective of refactoring page initialization and scanning logic to work withPageinstances.- 82-82: The refactoring of the
scanPagesmethod to work withPageinstances instead ofPagePathis a significant improvement. It simplifies the handling of pages by directly associating them with their respectivePageobjects. However, ensure that thegetPagePathsfunction is correctly updated to support this change.- 86-96: The
scanSubPagesmethod has been correctly refactored to work withPageinstances, similar to thescanPagesmethod. This consistency in handling both pages and subpages asPageinstances is beneficial for maintainability and clarity.- 180-180: The
parsePagesmethod's refactoring to work withPageinstances and its integration with type enhancements (e.g.,PageMetaDatum) is a positive change. It improves the clarity and type safety of the page parsing logic. Ensure that thePageclass'sgetOptionsmethod is implemented correctly and returns the expected data structure.- 249-261: The
getPageByPathmethod has been updated to retrieve pages by their absolute path, considering both main pages and subpages. This method is crucial for the dynamic update of pages JSON. Ensure that thePageclass'sfileproperty correctly stores theabsolutePathand is accessible as shown.- 265-266: The logic within
updatePagesJSONfor checking if a page has changed before proceeding with the update is a good performance optimization. However, ensure that thePageclass'shasChangedmethod is implemented correctly and efficiently determines if a page's content has changed.
edwinhuish
force-pushed
the
feat/support-definePage
branch
from
2ec3799 to
9acad28
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
packages/core/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (12)
- README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (11)
- README.md
- packages/core/client.d.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/core/src/utils.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/uni-pages.d.ts
Additional comments: 7
packages/core/src/context.ts (7)
- 21-21: The import of the
Pageclass is correctly added to support the newdefinePagefunctionality. Ensure that thePageclass is implemented as expected and integrates well with the existing codebase.- 43-44: The introduction of
pagesandsubPagesas class properties to store instances ofPageis a significant change. This approach aligns with the objective of supporting thedefinePagemacro by enabling more structured management of page definitions.- 82-82: Refactoring the
scanPagesmethod to work withPageinstances instead ofPagePathis a crucial change. It's important to ensure that thePageconstructor is called with the correct arguments and that thePageinstances are correctly managed within thePageContext.- 96-96: Similarly, the
scanSubPagesmethod has been refactored to createPageinstances for subpages. This consistency in handling pages and subpages asPageinstances is beneficial for maintainability and future enhancements.- 180-180: The
parsePagesmethod has been updated to work withPageinstances. It's crucial to verify thatpage.getOptions()correctly retrieves the options for each page and that the method correctly handles the merging of page metadata.- 249-261: The
getPageByPathmethod has been updated to retrievePageinstances by their absolute path. This change is essential for maintaining the integrity of page management. Ensure that the comparison ofabsolutePathis accurate and that the method correctly handles both main pages and subpages.- 265-266: The
updatePagesJSONmethod now includes logic to check if a page has changed before proceeding with the update. This optimization can help reduce unnecessary updates. However, it's important to ensure that thehasChangedmethod of thePageclass accurately detects changes.
edwinhuish
force-pushed
the
feat/support-definePage
branch
2 times, most recently
from
218af7d to
0d7e869
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
packages/core/package.jsonis excluded by:!**/*.jsonpackages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (18)
- README.md (1 hunks)
- packages/core/README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/child-process.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/import-module.vue (1 hunks)
- packages/playground/src/pages/macros/import.vue (1 hunks)
- packages/playground/src/pages/macros/nested-function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/pages/macros/utils.ts (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (11)
- README.md
- packages/core/client.d.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/core/src/utils.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/uni-pages.d.ts
Additional comments: 5
packages/playground/src/pages/macros/import-module.vue (1)
- 1-15: The usage of
definePageand the import ofversionfromviteare correctly implemented.packages/playground/src/pages/macros/nested-function.vue (1)
- 1-24: The usage of
definePagewith a nested functiongetTitleis correctly implemented and serves as a good example of incorporating more complex logic withindefinePage.packages/core/README.md (2)
- 47-69: The documentation for the
definePagemacro provides clear and accurate examples of how to define page options using both object and function arguments.- 44-74: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [71-100]
The documentation for adding route meta directly to routes using a custom
<route>block in SFCs is clear and informative, covering the supported parsers and how to specify a parser.packages/core/src/context.ts (1)
- 76-99: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-261]
The refactoring in
context.tsintroduces a more object-oriented approach to handling pages and sub-pages, improving maintainability and modularity. The changes appear to be correctly implemented.Please verify the performance impact of these changes, especially in projects with a large number of pages.
Comment on lines
+1
to
+18
| import type { SpawnOptionsWithoutStdio } from 'node:child_process' | ||
| import { spawn } from 'node:child_process' | ||
| import process from 'node:process' | ||
|
|
||
| export function runProcess(command: string, args: string[] = [], options?: SpawnOptionsWithoutStdio) { | ||
| return new Promise<string>((resolve, reject) => { | ||
| const child = spawn(command, args, { | ||
| env: { | ||
| ...process.env, | ||
| }, | ||
| ...options, | ||
| }) | ||
| const output = [] as string[] | ||
| child.stdout.on('data', chunk => output.push(chunk)) | ||
| child.on('close', () => resolve(output.join('').trim())) | ||
| child.on('error', error => reject(error)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The runProcess function correctly spawns a child process and handles success cases. However, it currently does not handle stderr output from the child process, which could lead to missed error messages. Consider adding a listener for stderr similar to stdout.
+ child.stderr.on('data', chunk => output.push(chunk))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| import type { SpawnOptionsWithoutStdio } from 'node:child_process' | |
| import { spawn } from 'node:child_process' | |
| import process from 'node:process' | |
| export function runProcess(command: string, args: string[] = [], options?: SpawnOptionsWithoutStdio) { | |
| return new Promise<string>((resolve, reject) => { | |
| const child = spawn(command, args, { | |
| env: { | |
| ...process.env, | |
| }, | |
| ...options, | |
| }) | |
| const output = [] as string[] | |
| child.stdout.on('data', chunk => output.push(chunk)) | |
| child.on('close', () => resolve(output.join('').trim())) | |
| child.on('error', error => reject(error)) | |
| }) | |
| } | |
| import type { SpawnOptionsWithoutStdio } from 'node:child_process' | |
| import { spawn } from 'node:child_process' | |
| import process from 'node:process' | |
| export function runProcess(command: string, args: string[] = [], options?: SpawnOptionsWithoutStdio) { | |
| return new Promise<string>((resolve, reject) => { | |
| const child = spawn(command, args, { | |
| env: { | |
| ...process.env, | |
| }, | |
| ...options, | |
| }) | |
| const output = [] as string[] | |
| child.stdout.on('data', chunk => output.push(chunk)) | |
| child.stderr.on('data', chunk => output.push(chunk)) | |
| child.on('close', () => resolve(output.join('').trim())) | |
| child.on('error', error => reject(error)) | |
| }) | |
| } |
edwinhuish
force-pushed
the
feat/support-definePage
branch
2 times, most recently
from
f540677 to
109d430
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
packages/core/package.jsonis excluded by:!**/*.jsonpackages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (17)
- README.md (1 hunks)
- packages/core/README.md (1 hunks)
- packages/core/src/child-process.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/import-module.vue (1 hunks)
- packages/playground/src/pages/macros/import.vue (1 hunks)
- packages/playground/src/pages/macros/nested-function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/pages/macros/utils.ts (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (15)
- README.md
- packages/core/README.md
- packages/core/src/child-process.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/import-module.vue
- packages/playground/src/pages/macros/import.vue
- packages/playground/src/pages/macros/nested-function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/pages/macros/utils.ts
- packages/playground/src/uni-pages.d.ts
Additional comments: 6
packages/core/src/utils.ts (2)
- 2-2: The import statement for
ModuleNodeandViteDevServerhas been updated to use type syntax, which is a good practice for TypeScript to ensure these imports are used for type information only and not included in the JavaScript output.- 60-75: The
parseSFCfunction is a new addition that utilizes@vue/compiler-sfcfor parsing Vue Single File Components (SFCs). It's important to handle errors gracefully, and the catch block does this by throwing a new error with a helpful message. However, consider adding more specific error handling or logging for different types of errors that could occur during parsing.Consider enhancing error handling in the
parseSFCfunction to provide more specific feedback for different error scenarios, which could aid in debugging.packages/core/src/context.ts (4)
- 21-21: The import of the
Pageclass is a key addition, indicating a shift towards using this class for page management. This aligns with the PR's objective of enhancing page definition capabilities.- 43-45: The introduction of
pagesandsubPagesas class properties to store instances of thePageclass is a significant change. It's crucial to ensure that all methods interacting with these properties are updated accordingly.- 82-82: The
scanPagesmethod has been refactored to createPageinstances for each path. This is a crucial part of integrating thePageclass into the system. Ensure that thePageconstructor and its methods are thoroughly tested to handle various page definitions correctly.- 180-180: The
parsePagesmethod has been updated to work withPageinstances. This method is central to processing page definitions and generating metadata. It's important to verify that thegetOptionsmethod of thePageclass correctly handles different page configurations.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
packages/core/package.jsonis excluded by:!**/*.jsonpackages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (17)
- README.md (1 hunks)
- packages/core/README.md (1 hunks)
- packages/core/src/child-process.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (4 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/import-module.vue (1 hunks)
- packages/playground/src/pages/macros/import.vue (1 hunks)
- packages/playground/src/pages/macros/nested-function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/pages/macros/utils.ts (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (16)
- README.md
- packages/core/README.md
- packages/core/src/child-process.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/core/src/utils.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/import-module.vue
- packages/playground/src/pages/macros/import.vue
- packages/playground/src/pages/macros/nested-function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/pages/macros/utils.ts
- packages/playground/src/uni-pages.d.ts
Additional comments: 7
packages/core/src/context.ts (7)
- 21-21: The import of the
Pageclass is correctly added to support the new functionality introduced by thedefinePagemacro. This aligns with the PR objectives.- 43-44: The declarations of
pagesandsubPagesas class properties are correctly refactored to use instances of thePageclass instead ofPagePath. This change is necessary for the enhanced page handling logic.- 82-82: The refactoring of the
scanPagesmethod to initializePageinstances for each path is correctly implemented. This change is crucial for the new page handling logic that leverages thePageclass.- 92-92: Similarly, the
scanSubPagesmethod is correctly refactored to createPageinstances for subpages. This ensures consistency in handling both main pages and subpages.- 180-180: The
parsePagesmethod's refactoring to work withPageinstances instead ofPagePathis correctly done. This method now correctly awaits the resolution ofpage.getOptions()for each page, which is essential for asynchronous page option retrieval.- 249-261: The
getPageByPathmethod is updated to retrievePageinstances by their absolute path. This update is necessary for the new page handling logic and correctly implemented.- 265-266: The
updatePagesJSONmethod's logic to check if a page has changed before updating the pages JSON is correctly implemented. This optimization prevents unnecessary updates, aligning with best practices for performance.
edwinhuish
force-pushed
the
feat/support-definePage
branch
3 times, most recently
from
22995a9 to
e532f7f
Compare
edwinhuish
force-pushed
the
feat/support-definePage
branch
from
e532f7f to
ee67edd
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
packages/core/package.jsonis excluded by:!**/*.jsonpackages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (19)
- README.md (1 hunks)
- packages/core/README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/child-process.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (2 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/import-module.vue (1 hunks)
- packages/playground/src/pages/macros/import.vue (1 hunks)
- packages/playground/src/pages/macros/nested-function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/pages/macros/remove-console.vue (1 hunks)
- packages/playground/src/pages/macros/utils.ts (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (17)
- README.md
- packages/core/README.md
- packages/core/client.d.ts
- packages/core/src/child-process.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/core/src/utils.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/import-module.vue
- packages/playground/src/pages/macros/import.vue
- packages/playground/src/pages/macros/nested-function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/pages/macros/utils.ts
- packages/playground/src/uni-pages.d.ts
Additional comments: 9
packages/playground/src/pages/macros/remove-console.vue (2)
- 3-3: The comment indicates that the console log statement should be removed. If this is for demonstration purposes related to the
definePagemacro's ability to handle or remove console logs, please ensure this is clearly documented to avoid confusion.- 12-14: The template block is straightforward and serves its purpose as a placeholder for page content.
packages/core/src/context.ts (7)
- 21-21: The addition of the
Pageimport aligns with the PR's objectives to enhance the development experience by usingPageinstances for better handling of page functionalities.- 43-44: The declaration of
pagesandsubPagesas arrays ofPageinstances supports the new approach of handling pages as objects, facilitating more sophisticated operations on pages.- 79-82: The refactoring of the
scanPagesmethod to directly createPageinstances simplifies the process and aligns with the object-oriented approach of handling pages.- 86-96: The refactoring of the
scanSubPagesmethod to work withPageinstances instead of paths simplifies the handling of sub-pages and enhances the object-oriented design.- 180-180: The update to the
parsePagesmethod to work withPageinstances instead of paths is a necessary change that improves the flexibility and robustness of page handling.- 220-220: The
mergePageMetaDatamethod's update to work withPageinstances supports the new approach and enhances the process of merging page metadata.- 249-249: The
getPageByPathmethod's update to retrievePageinstances by path aligns with the new approach of handling pages as objects, improving the overall design.
edwinhuish
force-pushed
the
feat/support-definePage
branch
from
ee67edd to
54d9c8b
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
packages/core/package.jsonis excluded by:!**/*.jsonpackages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (19)
- README.md (1 hunks)
- packages/core/README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/child-process.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (7 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (3 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/import-module.vue (1 hunks)
- packages/playground/src/pages/macros/import.vue (1 hunks)
- packages/playground/src/pages/macros/nested-function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/pages/macros/remove-console.vue (1 hunks)
- packages/playground/src/pages/macros/utils.ts (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (17)
- README.md
- packages/core/README.md
- packages/core/client.d.ts
- packages/core/src/child-process.ts
- packages/core/src/constant.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/import-module.vue
- packages/playground/src/pages/macros/import.vue
- packages/playground/src/pages/macros/nested-function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/pages/macros/remove-console.vue
- packages/playground/src/pages/macros/utils.ts
- packages/playground/src/uni-pages.d.ts
Additional comments: 7
packages/core/src/utils.ts (2)
- 2-2: The change to use type imports for
ModuleNodeandViteDevServerfrom 'vite' is a good practice for TypeScript, as it ensures these imports are used only for type checking and do not contribute to the runtime bundle size.- 61-76: The addition of the
parseSFCfunction is a significant enhancement. It encapsulates the parsing logic for Single File Components (SFCs) using@vue/compiler-sfc, with error handling that throws a more informative error if the parsing fails. This function improves maintainability by centralizing SFC parsing logic and error handling. However, consider adding more specific error handling or logging to provide clearer insights into parsing failures.Consider enhancing error handling within the
parseSFCfunction to log specific parsing errors or to categorize errors based on their nature. This could help developers diagnose issues more effectively.packages/core/src/context.ts (5)
- 21-21: The import of the
Pageclass aligns with the PR's objective to introduce a more structured approach to page management. This change suggests a shift towards object-oriented management of pages, which can enhance code readability and maintainability.- 43-45: The introduction of
pagesandsubPagesas class properties, with their types being arrays ofPageinstances or records ofPagearrays, respectively, is a significant improvement. It indicates a move towards a more structured and scalable way of handling pages and subpages within the project. This change should facilitate easier manipulation and querying of page data throughout the application.- 82-82: The refactoring to initialize
Pageinstances directly within thescanPagesmethod simplifies the process of page scanning and instantiation. This approach encapsulates the creation and initialization of pages within a single, cohesive operation, improving code clarity and maintainability.- 180-181: The
parsePagesmethod's asynchronous nature and its use ofPromise.allto concurrently process page options is an efficient way to handle potentially time-consuming operations. This method demonstrates good use of asynchronous programming patterns to improve performance.- 249-261: The
getPageByPathmethod provides a clear and concise way to retrieve a page by its absolute path, searching through both main pages and subpages. This utility method enhances code reusability and simplifies the logic for finding pages, contributing to better code organization.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
packages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (2)
- packages/playground/src/pages/macros/yaml.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/playground/src/uni-pages.d.ts
Additional comments: 3
packages/playground/src/pages/macros/yaml.vue (3)
- 5-11: The usage of
definePagehere demonstrates its capability to support functions, aligning with the PR objectives. Ensure thatdefinePageoperates in a separate scope from the page script as intended.- 3-3: The import of
yamlParseris correctly implemented. Given the PR's enhancements onimportsupport, ensure that this import does not introduce any unintended side effects or scope issues within thedefinePagemacro.- 18-20: The template is simple and effective for demonstrating the component's functionality in the playground. Good to go!
edwinhuish
force-pushed
the
feat/support-definePage
branch
from
cb7ec63 to
db9b6b9
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
packages/core/package.jsonis excluded by:!**/*.jsonpackages/playground/package.jsonis excluded by:!**/*.jsonpnpm-lock.yamlis excluded by:!**/*.yaml
Files selected for processing (20)
- README.md (1 hunks)
- packages/core/README.md (1 hunks)
- packages/core/client.d.ts (1 hunks)
- packages/core/src/child-process.ts (1 hunks)
- packages/core/src/constant.ts (1 hunks)
- packages/core/src/context.ts (9 hunks)
- packages/core/src/index.ts (3 hunks)
- packages/core/src/page.ts (1 hunks)
- packages/core/src/types.ts (3 hunks)
- packages/core/src/utils.ts (3 hunks)
- packages/playground/src/pages/macros/async-function.vue (1 hunks)
- packages/playground/src/pages/macros/function.vue (1 hunks)
- packages/playground/src/pages/macros/import-module.vue (1 hunks)
- packages/playground/src/pages/macros/import.vue (1 hunks)
- packages/playground/src/pages/macros/nested-function.vue (1 hunks)
- packages/playground/src/pages/macros/object.vue (1 hunks)
- packages/playground/src/pages/macros/remove-console.vue (1 hunks)
- packages/playground/src/pages/macros/utils.ts (1 hunks)
- packages/playground/src/pages/macros/yaml.vue (1 hunks)
- packages/playground/src/uni-pages.d.ts (2 hunks)
Files skipped from review as they are similar to previous changes (19)
- README.md
- packages/core/README.md
- packages/core/client.d.ts
- packages/core/src/child-process.ts
- packages/core/src/constant.ts
- packages/core/src/context.ts
- packages/core/src/index.ts
- packages/core/src/page.ts
- packages/core/src/types.ts
- packages/playground/src/pages/macros/async-function.vue
- packages/playground/src/pages/macros/function.vue
- packages/playground/src/pages/macros/import-module.vue
- packages/playground/src/pages/macros/import.vue
- packages/playground/src/pages/macros/nested-function.vue
- packages/playground/src/pages/macros/object.vue
- packages/playground/src/pages/macros/remove-console.vue
- packages/playground/src/pages/macros/utils.ts
- packages/playground/src/pages/macros/yaml.vue
- packages/playground/src/uni-pages.d.ts
Additional comments: 1
packages/core/src/utils.ts (1)
- 2-2: The change to use type imports for
ModuleNodeandViteDevServerfrom 'vite' is a good practice for TypeScript, as it clarifies that these imports are only used for type information and can be stripped away during compilation.
Comment on lines
+61
to
+76
| export async function parseSFC(code: string, options?: SFCParseOptions): Promise<SFCDescriptor> { | ||
| try { | ||
| return ( | ||
| VueParser(code, { | ||
| pad: 'space', | ||
| ...options, | ||
| }).descriptor | ||
| // for @vue/compiler-sfc ^2.7 | ||
| || (VueParser as any)({ | ||
| source: code, | ||
| ...options, | ||
| }) | ||
| ) | ||
| } | ||
|
|
||
| async function hasChanged(filePath: string, routeBlock?: SFCBlock) { | ||
| if (!routeBlock) | ||
| routeBlock = await getRouteSfcBlock(normalizePath(filePath)) | ||
|
|
||
| return !pages.has(filePath) || JSON.stringify(parseData(routeBlock)) !== pages.get(filePath) | ||
| } | ||
|
|
||
| return { | ||
| setCache, | ||
| hasChanged, | ||
| catch (error) { | ||
| throw new Error(`[vite-plugin-uni-pages] Vue3's "@vue/compiler-sfc" is required. \nOriginal error: \n${error}`) |
There was a problem hiding this comment.
The parseSFC function is a valuable addition for handling SFC parsing with proper error handling. However, when re-throwing the error, consider preserving the original error stack for easier debugging.
- throw new Error(`[vite-plugin-uni-pages] Vue3's "@vue/compiler-sfc" is required. \nOriginal error: \n${error}`)
+ const newError = new Error(`[vite-plugin-uni-pages] Vue3's "@vue/compiler-sfc" is required. \nOriginal error: \n${error}`);
+ newError.stack = error.stack;
+ throw newError;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| export async function parseSFC(code: string, options?: SFCParseOptions): Promise<SFCDescriptor> { | |
| try { | |
| return ( | |
| VueParser(code, { | |
| pad: 'space', | |
| ...options, | |
| }).descriptor | |
| // for @vue/compiler-sfc ^2.7 | |
| || (VueParser as any)({ | |
| source: code, | |
| ...options, | |
| }) | |
| ) | |
| } | |
| async function hasChanged(filePath: string, routeBlock?: SFCBlock) { | |
| if (!routeBlock) | |
| routeBlock = await getRouteSfcBlock(normalizePath(filePath)) | |
| return !pages.has(filePath) || JSON.stringify(parseData(routeBlock)) !== pages.get(filePath) | |
| } | |
| return { | |
| setCache, | |
| hasChanged, | |
| catch (error) { | |
| throw new Error(`[vite-plugin-uni-pages] Vue3's "@vue/compiler-sfc" is required. \nOriginal error: \n${error}`) | |
| export async function parseSFC(code: string, options?: SFCParseOptions): Promise<SFCDescriptor> { | |
| try { | |
| return ( | |
| VueParser(code, { | |
| pad: 'space', | |
| ...options, | |
| }).descriptor | |
| // for @vue/compiler-sfc ^2.7 | |
| || (VueParser as any)({ | |
| source: code, | |
| ...options, | |
| }) | |
| ) | |
| } | |
| catch (error) { | |
| const newError = new Error(`[vite-plugin-uni-pages] Vue3's "@vue/compiler-sfc" is required. \nOriginal error: \n${error}`); | |
| newError.stack = error.stack; | |
| throw newError; |
|
Sorry，最近大家可能都比较忙，可能没有时间看，如果非必要请不要 @，感谢。 如果比较着急，可以看自己发个包用一下。 我看你的文件改动比较大，如果不冲突最好把重构和新功能分开 pr。 |
我最近没有用uniapp的需求。如果有其他人需要这个功能再考虑另发包。
改动的细分很难界定，曾经试过在其他库提几个文件的修改，因为代码样式、实现方式等，来来回回改近十个commit，拖了十多天才通过第一个PR，后续PR都一直等第一个PR合并才能继续。。。 另，正如我之前所说，我写这个PR，仅仅是为爱发电，如果你觉得改动太多，不好梳理，或者你们按照需求去拆分commit？对于commit的owner是否我，都没问题。 此 PR 暂关闭，分支在此仓库下，你们自行决定怎么处理吧？ |
|
个人感觉definePage比其他几种方式更优雅 希望早日能实现合并 |
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
支持
definePage宏 related #134playground下的示例import使用注意：
import仅支持从js或ts导入，不支持从vue文件里导入import导入暂不支持别名路径definePage和页面script不在同一个作用域，无法使用页面变量。实现原理：
使用
@vue/compiler-sfc和@babel/types获取宏代码节点vite-plugin-uni-pages/packages/core/src/page.ts
Lines 86 to 88 in 218af7d
使用
@babel/generator还原为 JS codevite-plugin-uni-pages/packages/core/src/page.ts
Lines 99 to 101 in 218af7d
创建子进程，并调用
tsx运行生成的 script，得到结果。vite-plugin-uni-pages/packages/core/src/page.ts
Lines 201 to 212 in 0d7e869
Summary by CodeRabbit
New Features
definePagefunction for more flexible page definitions.Refactor
Pageinstances, improving the handling of page options.Bug Fixes
Chores
child-process.tsfile for running child processes, facilitating external command execution.Documentation