-
Notifications
You must be signed in to change notification settings - Fork 38
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
refactor: Build split code #55
Conversation
WalkthroughThis pull request introduces several changes across multiple files, primarily focusing on the addition of an SCSS stylesheet import for the Changes
Possibly related PRs
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
Files selected for processing (27)
- README.md (1 hunks)
- README.zh-CN.md (1 hunks)
- packages/docs/fluent-editor/demos/basic-usage.vue (1 hunks)
- packages/docs/fluent-editor/demos/code-block-highlight.vue (1 hunks)
- packages/docs/fluent-editor/demos/custom-toolbar.vue (1 hunks)
- packages/docs/fluent-editor/demos/emoji.vue (1 hunks)
- packages/docs/fluent-editor/demos/file-upload.vue (1 hunks)
- packages/docs/fluent-editor/demos/format-painter.vue (1 hunks)
- packages/docs/fluent-editor/demos/formula.vue (1 hunks)
- packages/docs/fluent-editor/demos/image-upload-to-server.vue (1 hunks)
- packages/docs/fluent-editor/demos/image-upload.vue (1 hunks)
- packages/docs/fluent-editor/demos/markdown.vue (1 hunks)
- packages/docs/fluent-editor/demos/mention-custom-list.vue (1 hunks)
- packages/docs/fluent-editor/demos/mention.vue (1 hunks)
- packages/docs/fluent-editor/demos/table.vue (1 hunks)
- packages/docs/fluent-editor/docs/quick-start.md (2 hunks)
- packages/docs/fluent-editor/vite.config.ts (1 hunks)
- packages/docs/package.json (1 hunks)
- packages/fluent-editor/package.json (1 hunks)
- packages/fluent-editor/scripts/pre-release.js (1 hunks)
- packages/fluent-editor/src/assets/index.scss (1 hunks)
- packages/fluent-editor/src/index.ts (1 hunks)
- packages/fluent-editor/src/screenshot/index.ts (3 hunks)
- packages/fluent-editor/src/syntax/index.ts (1 hunks)
- packages/fluent-editor/src/toolbar/index.ts (1 hunks)
- packages/fluent-editor/vite.config.theme.ts (1 hunks)
- packages/fluent-editor/vite.config.ts (1 hunks)
Files skipped from review due to trivial changes (17)
- README.md
- packages/docs/fluent-editor/demos/basic-usage.vue
- packages/docs/fluent-editor/demos/code-block-highlight.vue
- packages/docs/fluent-editor/demos/custom-toolbar.vue
- packages/docs/fluent-editor/demos/emoji.vue
- packages/docs/fluent-editor/demos/file-upload.vue
- packages/docs/fluent-editor/demos/format-painter.vue
- packages/docs/fluent-editor/demos/formula.vue
- packages/docs/fluent-editor/demos/image-upload-to-server.vue
- packages/docs/fluent-editor/demos/image-upload.vue
- packages/docs/fluent-editor/demos/markdown.vue
- packages/docs/fluent-editor/demos/mention-custom-list.vue
- packages/docs/fluent-editor/demos/mention.vue
- packages/docs/fluent-editor/demos/table.vue
- packages/fluent-editor/src/assets/index.scss
- packages/fluent-editor/src/index.ts
- packages/fluent-editor/vite.config.theme.ts
Additional context used
Biome
packages/fluent-editor/src/syntax/index.ts
[error] 6-9: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
Additional comments not posted (28)
packages/docs/fluent-editor/vite.config.ts (1)
3-19
: LGTM!The changes introduce a new configuration for module resolution using path aliases, which enhances the modularity and maintainability of the code by simplifying the import paths for developers. The
path
module is correctly used to resolve paths relative to thefluent-editor
directory, and the alias mappings are properly defined under theresolve
property.packages/docs/package.json (1)
23-23
: LGTM!The changes introduce new dependencies:
quill
,highlight.js
, andhtml2canvas
, which are correctly added to thedependencies
section ofpackage.json
. These additions suggest an enhancement in the project's capabilities, likely aimed at improving text editing, syntax highlighting, and rendering of HTML elements on the canvas.Also applies to: 27-28
packages/fluent-editor/src/syntax/index.ts (1)
2-2
: LGTM!The changes modify the way the
Syntax
module is imported and utilized within theCustomSyntax
class. The import statement for thehighlight.js
library has been removed, and instead, theTypeSyntax
module fromquill/modules/syntax
is imported. This change alters the method of accessing theSyntax
class, switching fromQuill.imports['modules/syntax']
toQuill.import('modules/syntax') as typeof TypeSyntax
, which may enhance type safety and clarity in the code.Additionally, the
DEFAULTS
property of theCustomSyntax
class has been updated to assign a function that retrieveswindow.hljs
to thehljs
property. This change suggests a shift towards a more dynamic approach for accessing thehljs
library, potentially allowing for better integration with the global scope and improving compatibility with different environments.Overall, these changes reflect a refinement in the module's structure and initialization, enhancing the clarity and functionality of the
CustomSyntax
class.Also applies to: 4-4, 12-15
packages/docs/fluent-editor/docs/quick-start.md (2)
27-27
: LGTM!The import of the SCSS file is necessary to ensure the editor is styled correctly.
62-62
: LGTM!The import of the SCSS file is necessary to ensure the editor is styled correctly.
packages/fluent-editor/scripts/pre-release.js (5)
31-31
: LGTM!The change to the
main
entry is valid and is likely aimed at improving the project's build or distribution structure.
32-32
: LGTM!The change to the
module
entry is valid and is likely aimed at improving the project's build or distribution structure.
33-33
: LGTM!The change to the
import
entry is valid and is likely aimed at improving the project's build or distribution structure.
34-34
: LGTM!The change to the
require
entry is valid and is likely aimed at improving the project's build or distribution structure.
35-35
: LGTM!The change to the mapping of
./index.scss
to./theme/index.css
is valid and is likely aimed at improving the organization of styles in the project.packages/fluent-editor/package.json (5)
32-32
: LGTM!The addition of the new entry for
"./index.scss"
pointing to"./src/assets/index.scss"
is valid and is likely aimed at improving the organization and maintainability of styles in the project.
37-37
: LGTM!The modification of the
build
script to include the new commandpnpm build:theme
is valid and is likely aimed at improving the modularity and maintainability of the build process.
38-38
: LGTM!The addition of the new
build:theme
script that utilizes a specific Vite configuration filevite.config.theme.ts
is valid and is likely aimed at improving the modularity and maintainability of the build process.
42-44
: LGTM!The introduction of the new
peerDependencies
section specifying"quill": "^2.0.0"
is valid and is likely aimed at improving the dependency management of the project.
59-59
: LGTM, but verify the Vite upgrade.The upgrade of the
vite
version from^2.3.0
to^5.0.0
is valid and is likely aimed at improving the build process of the project.However, ensure that this upgrade is thoroughly tested to confirm that it does not introduce any breaking changes or compatibility issues.
Run the following script to verify the Vite upgrade:
packages/fluent-editor/vite.config.ts (5)
6-10
: LGTM!The new
Manifest
interface provides a structured way to manage package manifests, including versioning and optional dependency records. This change improves the maintainability and readability of the code.
11-13
: LGTM!The new
getPackageManifest
function reads and parses a package's manifest file, returning an object that conforms to theManifest
interface. This change streamlines the handling of dependencies and improves the modularity of the code.
14-23
: LGTM!The new
rollupExternalFromPackage
function dynamically determines external dependencies based on the manifest. It constructs a set of dependency keys and checks if a given module ID matches any of these keys or starts with them. This change enhances the control over which packages are treated as external during bundling and improves the flexibility of the module resolution during the build process.
25-32
: LGTM!The new
rollupOutput
function provides a more comprehensive setup for Rollup output options, supporting both ES and CommonJS formats, with specific settings for file naming and directory structure. This change improves the modularity and maintainability of the build process.
34-37
: LGTM!The input files are now dynamically gathered using
glob
, allowing for a more scalable approach to include all TypeScript files in the source directory. This change improves the flexibility and maintainability of the build configuration.README.zh-CN.md (1)
45-45
: LGTM!The addition of the stylesheet import is crucial for the visual presentation of the editor, as it applies the necessary styles defined in the SCSS file. This change enhances the functionality by ensuring that the editor is styled according to the specified theme.
packages/fluent-editor/src/screenshot/index.ts (6)
4-5
: LGTM!The import of
Delta
fromquill
is necessary for the functionality of theScreenshot
class. This change is a minor refactoring that improves the readability and maintainability of the code.
6-9
: LGTM!The
ScreenShotOptions
interface allows for the possibility of passing a custom implementation ofHtml2Canvas
, thereby increasing the module's adaptability. This change improves the robustness and configurability of theScreenshot
module, making it more versatile for different use cases.
12-18
: LGTM!The update to the
DEFAULTS
static property ensures that theHtml2Canvas
library is available when the module is used. This change improves the robustness of theScreenshot
module by providing a fallback mechanism for initializingHtml2Canvas
.
32-37
: LGTM!The modification to the constructor enhances type safety and clarity by accepting a
Partial<ScreenShotOptions>
type for the options parameter. The error handling enforces the requirement for theHtml2Canvas
library, thereby improving the robustness of theScreenshot
module.
135-135
: LGTM!The update to the usage of
Html2Canvas
ensures that the method utilizes the potentially overriddenHtml2Canvas
function from theoptions
property. This change improves the flexibility and configurability of theScreenshot
module by allowing for custom implementations ofHtml2Canvas
.
138-138
: LGTM!The update to the usage of
Html2Canvas
ensures that the method utilizes the potentially overriddenHtml2Canvas
function from theoptions
property. This change improves the flexibility and configurability of theScreenshot
module by allowing for custom implementations ofHtml2Canvas
.packages/fluent-editor/src/toolbar/index.ts (1)
6-6
: LGTM!The change of importing the
Delta
class directly from the Quill instance instead of relying on thequill-delta
package is approved.This change enhances compatibility with the Quill library's internal structure and potentially reduces dependencies on external packages while preserving the functionality associated with
Delta
.
@zzxming 代码块高亮的 demo 有报错,功能失效 |
@zzxming 这次重构非常赞!不仅解决了 Quill 内容导出问题,抽离了样式,而且优化了产物结构,减少了包体积。 可能有个小问题,之前引入样式的方式:
|
@zzxming 还发现两个问题,一个是插入表情功能失效,点击工具栏的表情图标不会弹出表情面板 |
有复现方式吗?我通过下载 |
文档说明已修改,可正常使用 |
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
packages/docs/fluent-editor/docs/code-block-highlight.md (1)
11-12
: Remove the empty lines.The two empty lines at the end of the file do not serve any purpose. Please remove them to keep the documentation concise.
Apply this diff to remove the empty lines:
- -packages/fluent-editor/src/config/types/editor-modules.interface.ts (1)
23-29
: LGTM! Consider adding JSDoc comments for better documentation.The changes to the
syntax
property of theIEditorModules
interface are approved. The new structure allows for more flexible configuration of the syntax highlighting feature, which is a valuable enhancement.To improve the maintainability and usability of the code, consider adding JSDoc comments to document the new properties and their expected values. This will help future developers understand the purpose and usage of each property.
For example:
/** * Configuration for syntax highlighting. * @property {number} [interval] - The interval (in milliseconds) at which syntax highlighting should be updated. * @property {{key: string, label: string}[]} [languages] - The list of supported languages, each with a unique key and a display label. * @property {any} [hljs] - Additional configuration options to pass to the syntax highlighting library. */ syntax?: { interval?: number languages?: { key: string, label: string }[] hljs?: any } | boolean
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (26)
- README.md (1 hunks)
- README.zh-CN.md (1 hunks)
- packages/docs/fluent-editor/demos/basic-usage.vue (1 hunks)
- packages/docs/fluent-editor/demos/code-block-highlight.vue (2 hunks)
- packages/docs/fluent-editor/demos/counter-count.vue (1 hunks)
- packages/docs/fluent-editor/demos/counter.vue (1 hunks)
- packages/docs/fluent-editor/demos/custom-toolbar.vue (1 hunks)
- packages/docs/fluent-editor/demos/emoji.vue (1 hunks)
- packages/docs/fluent-editor/demos/file-upload.vue (1 hunks)
- packages/docs/fluent-editor/demos/format-painter.vue (1 hunks)
- packages/docs/fluent-editor/demos/formula.vue (1 hunks)
- packages/docs/fluent-editor/demos/get-html.vue (1 hunks)
- packages/docs/fluent-editor/demos/image-upload-to-server.vue (1 hunks)
- packages/docs/fluent-editor/demos/image-upload.vue (1 hunks)
- packages/docs/fluent-editor/demos/markdown.vue (1 hunks)
- packages/docs/fluent-editor/demos/mention-custom-list.vue (1 hunks)
- packages/docs/fluent-editor/demos/mention.vue (1 hunks)
- packages/docs/fluent-editor/demos/table.vue (1 hunks)
- packages/docs/fluent-editor/demos/video.vue (1 hunks)
- packages/docs/fluent-editor/docs/code-block-highlight.md (1 hunks)
- packages/docs/fluent-editor/docs/quick-start.md (2 hunks)
- packages/fluent-editor/package.json (1 hunks)
- packages/fluent-editor/scripts/pre-release.js (1 hunks)
- packages/fluent-editor/src/assets/style.scss (1 hunks)
- packages/fluent-editor/src/config/types/editor-modules.interface.ts (1 hunks)
- packages/fluent-editor/vite.config.theme.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- packages/docs/fluent-editor/demos/counter-count.vue
- packages/docs/fluent-editor/demos/counter.vue
- packages/docs/fluent-editor/demos/video.vue
Files skipped from review as they are similar to previous changes (18)
- README.md
- README.zh-CN.md
- packages/docs/fluent-editor/demos/basic-usage.vue
- packages/docs/fluent-editor/demos/custom-toolbar.vue
- packages/docs/fluent-editor/demos/emoji.vue
- packages/docs/fluent-editor/demos/file-upload.vue
- packages/docs/fluent-editor/demos/format-painter.vue
- packages/docs/fluent-editor/demos/formula.vue
- packages/docs/fluent-editor/demos/image-upload-to-server.vue
- packages/docs/fluent-editor/demos/image-upload.vue
- packages/docs/fluent-editor/demos/markdown.vue
- packages/docs/fluent-editor/demos/mention-custom-list.vue
- packages/docs/fluent-editor/demos/mention.vue
- packages/docs/fluent-editor/demos/table.vue
- packages/docs/fluent-editor/docs/quick-start.md
- packages/fluent-editor/package.json
- packages/fluent-editor/scripts/pre-release.js
- packages/fluent-editor/vite.config.theme.ts
Additional comments not posted (10)
packages/docs/fluent-editor/docs/code-block-highlight.md (1)
3-7
: LGTM!The updated instructions for enabling code block highlighting using
highlight.js
are clear and cover both scenarios effectively. The formatting and link to the library's website enhance the documentation.packages/fluent-editor/src/assets/style.scss (1)
1-37
: LGTM!The code changes in this new SCSS file are well-structured and modularized. The font-face declaration is correctly defined, and the order of imports is logical. The use of
@include
directives promotes code reusability and modularity. There are no apparent issues or inconsistencies in the code.The code changes are approved.
packages/docs/fluent-editor/demos/code-block-highlight.vue (4)
4-4
: LGTM!The import statement for the SCSS stylesheet is correct and necessary for styling the
FluentEditor
component.
5-10
: LGTM!The import statements for
highlight.js
, the theme, and the Go language module are correct. Registering the Go language module is necessary for enabling syntax highlighting for Go code.
31-36
: LGTM!The configuration of the
syntax
module is correct. Specifyinghighlight.js
as the syntax highlighter and including Go as an additional language option is necessary for enabling syntax highlighting for Go code.
Line range hint
1-45
: Overall, the changes look great!The file imports the necessary dependencies, including the SCSS stylesheet,
highlight.js
, and the Go language module. The configuration of thesyntax
module is correct, enabling syntax highlighting for Go code usinghighlight.js
.The changes are well-structured, follow best practices, and enhance the functionality of the code block highlighting demo. I don't have any additional suggestions or issues to report.
Great work!
packages/docs/fluent-editor/demos/get-html.vue (4)
4-4
: LGTM!The code changes are approved. Importing the SCSS stylesheet for the
FluentEditor
component ensures that the editor is styled correctly.
7-8
: LGTM!The code changes are approved. Importing the
highlight.js
library and its corresponding CSS enhances the editor's capabilities to support code formatting and highlighting.
10-10
: LGTM!The code changes are approved. Setting the global variable
window.hljs
makes thehighlight.js
library accessible globally within the application.
9-9
: LGTM!The code changes are approved. Setting the global variable
window.katex
makes thekatex
library accessible globally within the application.
有的 https://stackblitz.com/edit/vitejs-vite-q9ttrh?file=src%2FApp.vue |
这是因为 Quill 初始化 |
是不是可以给个提示,如果配置了 syntax: true,就提示安装 highlight.js |
quill 本身在控制台是有提示的 |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
将
quill
作为peerDependencies
,用户需要自行安装,用于解决用户需要导出quill
中并没有在主文件导出的方法。这应该能解决 issue #46 ,用户可以自行从quill
中导出需要使用的方法将依赖的
highlight.js
、html2canvas
作为devDependencies
,这两项依赖仅在单独的模块中使用,当用户不使用对应模块时,这是多余的,当用户需要使用时自行安装,并作为模块的options
传入从入口文件的导入中移除 css 文件的导入,css 文件应该与 js 文件分离
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores