Skip to content
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

feat: tinyvue grid slot generate code to template #756

Open
wants to merge 2 commits into
base: refactor/develop
Choose a base branch
from

Conversation

chilingling
Copy link
Member

@chilingling chilingling commented Aug 26, 2024

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

【问题描述】
如 issue #754 所描述,当表格组件使用插槽并为插槽里面的组件添加样式时,页面预览以及下载代码后,为插槽里面的组件添加的样式无法生效。

【问题分析】
vue setup 中的 jsx,无法自动添加 scoped id,但是添加的样式却是默认是 scoped id 的。导致样式无法应用。

【解决方案】
tinyvue grid 组件有插槽时,不直接在 setup 组件中生成 jsx,而是将表格列配置作为 tiny-grid-column 生成到 template 中。

即从生成 jsx :

<template>
	<tiny-grid  :fetchData={ api: fetchData }  :columns="state.columns">
   </tiny-grid>
</template>
<script setup>
	const state = reactive({
        columns: [
        {
            field: 'city',
            slots: {
              default: ({ row }, h) => (
                //  这里的样式类不生效
                 <div  className="test-class">
                      { row.city }
                  </div>
              )
            }
         }
       ]
    })
</script>
<style scoped>
.test-class {
  color: red;
}
</style>

改成生成代码为:

<template>
	<tiny-grid  :fetchData={ api: fetchData } >
      <tiny-grid-column field="city">
          <template #default="{ row }">
              <div class="test-class">{{ row.city }}</div>
          </template>
     </tiny-grid-column>
   </tiny-grid>
</template>
<script setup>

</script>
<style scoped>
.test-class {
  color: red;
}
</style>

What is the current behavior?

Issue Number: #754

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Copy link
Contributor

coderabbitai bot commented Aug 26, 2024

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Cannot read config file: /packages/vue-generator/.eslintrc.cjs
Error: Cannot find module '@rushstack/eslint-patch/modern-module-resolution'
Require stack:

  • /packages/vue-generator/.eslintrc.cjs
  • /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/cli-engine/cli-engine.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/eslint.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/index.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/cli.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/bin/eslint.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Module._load (node:internal/modules/cjs/loader:1074:27)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Module.require (node:internal/modules/cjs/loader:1339:12)
    at require (node:internal/modules/helpers:135:16)
    at Object. (/packages/vue-generator/.eslintrc.cjs:4:1)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)

Walkthrough

This pull request introduces modifications to the Vue generator's component generation process, focusing on enhancing the handling of TinyGrid components and their columns. The changes span multiple files, including generator configurations, template generation, and test case updates. The primary modifications involve adjusting hooks, transforming slot and column handling, and updating component mappings to support more flexible grid column definitions.

Changes

File Change Summary
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js Moved handleTinyGrid from defaultAttributeHook to defaultComponentHooks
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js Added new functions: transformSlots, transformColumnToChildren, columnHasSlots; Updated import and slot handling logic
packages/vue-generator/test/testcases/sfc/case01/componentsMap.json Added new component mapping for TinyGridColumn
packages/vue-generator/test/testcases/sfc/case01/expected/FormTable.vue Restructured grid columns using explicit tiny-grid-column components
packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json Updated state structure to centralize column definitions

Sequence Diagram

sequenceDiagram
    participant Generator
    participant Template
    participant ComponentHooks
    participant Columns

    Generator->>ComponentHooks: Apply handleTinyGrid hook
    ComponentHooks->>Template: Transform slots
    Template->>Columns: Process column definitions
    Columns-->>Template: Return transformed columns
    Template-->>Generator: Generate component with new structure
Loading

Possibly related PRs

Suggested Labels

enhancement, bug

Suggested Reviewers

  • hexqi
  • rhlin

Poem

🐰 Hopping through code with glee,
Tiny Grid columns now set free!
Slots transformed, hooks rearranged,
Vue generation, beautifully changed!
A rabbit's dance of pure delight! 🎉

Finishing Touches

  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@chilingling chilingling force-pushed the feat/tinyvueGridGenCode branch from 7ee5602 to 1ba0e01 Compare January 8, 2025 07:49
@chilingling chilingling marked this pull request as ready for review January 13, 2025 12:54
@github-actions github-actions bot added enhancement New feature or request refactor-main refactor/develop branch feature labels Jan 13, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (4)

70-89: Robustness in transformSlots function

The transformSlots function effectively transforms slot configurations. However, consider adding a check to ensure that value within the slots is defined to prevent potential undefined issues in value?.params and value?.value.

Apply this diff to include default values:

        return {
          componentName: 'template',
          props: {
            slot: {
              name: key,
-             params: value?.params || ''
+             params: (value && value.params) || ''
            }
          },
-         children: value?.value
+         children: (value && value.value) || ''
        }

91-93: Consistent return values in transformColumnToChildren

When columns is not an array, the function returns undefined. For consistency and to avoid unexpected behavior in calling functions, consider returning an empty array instead.

Apply this diff:

-  if (!Array.isArray(columns)) {
-    return
-  }
+  if (!Array.isArray(columns)) {
+    return []
+  }

96-110: Avoid mutation of input objects

In transformColumnToChildren, you are destructuring item and modifying restItem. To prevent side effects, it's better to create a deep copy of item before modification.

Apply this diff to clone item:

  const res = columns.map((item) => {
+   const clonedItem = { ...item }
    const { slots, ...restItem } = clonedItem
    let children = []
    if (slots) {
      children = transformSlots(slots)
    }
    return {
      componentName: 'TinyGridColumn',
      props: restItem,
      children
    }
  })

116-123: Optimize columnHasSlots function

The columnHasSlots function can return early upon finding the first column with slots. It's already efficient, but you can use the some method for cleaner code.

Apply this diff:

-const columnHasSlots = (columns) => {
-  for (const columnItem of columns) {
-    if (columnItem.slots && typeof columnItem.slots === 'object' && Object.keys(columnItem.slots).length > 0) {
-      return true
-    }
-  }
-  return false
-}
+const columnHasSlots = (columns) => {
+  return columns.some(
+    (columnItem) =>
+      columnItem.slots &&
+      typeof columnItem.slots === 'object' &&
+      Object.keys(columnItem.slots).length > 0
+  )
+}
packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json (2)

15-48: Consider adding validation for numeric input.

The employees field uses TinyNumeric but lacks input validation beyond the step constraint.

Consider adding min/max constraints and validation:

  "componentName": "TinyNumeric",
  "props": {
    "allow-empty": true,
    "placeholder": "请输入",
    "controlsPosition": "right",
    "step": 1,
+   "min": 0,
+   "max": 999999,
    "modelValue": {

74-79: Implement proper change handlers.

The onChange handlers only log to console without actual implementation.

Would you like me to help implement proper change handlers for both numeric and text inputs? This should include:

  • Data validation
  • State updates
  • Error handling

Also applies to: 35-40

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a14a508 and 8fe57a9.

📒 Files selected for processing (6)
  • packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (1 hunks)
  • packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (3 hunks)
  • packages/vue-generator/test/testcases/sfc/case01/componentsMap.json (1 hunks)
  • packages/vue-generator/test/testcases/sfc/case01/expected/FormTable.vue (2 hunks)
  • packages/vue-generator/test/testcases/sfc/slotModelValue/expected/slotModelValueTest.vue (1 hunks)
  • packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/vue-generator/test/testcases/sfc/slotModelValue/expected/slotModelValueTest.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: push-check
🔇 Additional comments (7)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (2)

13-13: Ensure getImportMap is properly utilized

You have imported getImportMap at line 13, but please verify that it's correctly utilized in the code to prevent any unused imports.


158-175: Validate deletion of props.columns after transformation

After transforming props.columns to children components, you delete props.columns. Ensure that props.columns is not used elsewhere in the codebase after this deletion to prevent unintended side effects.

Run the following script to search for usages of props.columns after this point:

✅ Verification successful

props.columns deletion is properly implemented

The deletion of props.columns is safe as it occurs:

  • After all its usages are complete
  • Only when slots transformation is performed
  • Within a properly scoped context
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if `props.columns` is used after deletion in `handleTinyGrid`.

# Search for `props.columns` in files excluding `generateTemplate.js`
rg "props\.columns" --glob '!generateTemplate.js'

Length of output: 603


Script:

#!/bin/bash
# Check usage of props.columns within generateTemplate.js
rg "props\.columns" packages/vue-generator/src/generator/vue/sfc/generateTemplate.js -A 2 -B 2

# Check the function context where deletion occurs
ast-grep --pattern 'function $FUNC($_) {
  $$$
  delete props.columns
  $$$
}'

Length of output: 714

packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (1)

214-214: Confirm the placement of handleTinyGrid in defaultComponentHooks

Moving handleTinyGrid from defaultAttributeHook to defaultComponentHooks changes when this hook is executed. Ensure this change aligns with the intended hook execution order and that it doesn't affect other component processing logic.

packages/vue-generator/test/testcases/sfc/case01/expected/FormTable.vue (1)

57-58: Ensure TinyIconDel is correctly defined

Verify that TinyIconDel is properly imported and initialized to avoid runtime errors when rendering the icon.

packages/vue-generator/test/testcases/sfc/case01/componentsMap.json (1)

30-36: LGTM! Verify component version compatibility.

The addition of TinyGridColumn component follows the established patterns and aligns with the grid slot generation feature.

Run this script to verify the version compatibility with other @opentiny/vue components:

✅ Verification successful

Version compatibility verified ✓

The TinyGridColumn component uses version ^3.10.0, which is consistent with all other @opentiny/vue components in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check version compatibility across @opentiny/vue components
# Expect: All components should use the same version

rg -g '*.json' '"@opentiny/vue"' | grep '"version"' | sort | uniq -c

Length of output: 70


Script:

#!/bin/bash
# Find all componentsMap.json files and extract package + version info for @opentiny/vue
fd componentsMap.json --exec sh -c 'echo "=== $1 ==="; jq -r ".[] | select(.package == \"@opentiny/vue\") | \"\(.componentName): \(.version)\"" {}' sh {}

Length of output: 396

packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json (2)

2-90: LGTM! Well-structured state management for grid columns.

The move to state-based column management improves maintainability and reusability.


119-122: Implement error handling for undefined state.columns.

The JSExpression accessing state.columns lacks error handling.

Consider adding a default empty array to prevent runtime errors:

  "columns": {
    "type": "JSExpression",
-   "value": "state.columns"
+   "value": "state.columns || []"
  },

Comment on lines +94 to 96
GridColumn as TinyGridColumn,
Switch as TinySwitch
} from '@opentiny/vue'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Import statements for icons may be missing

In your import statements, ensure that all icons used in the template are imported from '@opentiny/vue-icon'. For example, IconDel should be imported to define TinyIconDel.

Apply this diff to include the missing imports:

 import {
   Button as TinyButton,
   Form as TinyForm,
   FormItem as TinyFormItem,
   Grid as TinyGrid,
   Input as TinyInput,
   Select as TinySelect,
   GridColumn as TinyGridColumn,
   Switch as TinySwitch
-} from '@opentiny/vue'
+} from '@opentiny/vue'
+import { IconSearch, IconDel, iconHelpCircle, IconEdit } from '@opentiny/vue-icon'
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
GridColumn as TinyGridColumn,
Switch as TinySwitch
} from '@opentiny/vue'
GridColumn as TinyGridColumn,
Switch as TinySwitch
} from '@opentiny/vue'
import { IconSearch, IconDel, iconHelpCircle, IconEdit } from '@opentiny/vue-icon'

Comment on lines +41 to +60
<tiny-grid :fetchData="{ api: getTableData }">
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column>
<tiny-grid-column type="selection" :width="60"></tiny-grid-column>
<tiny-grid-column field="employees" title="员工数">
<template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template
></tiny-grid-column>
<tiny-grid-column field="city" title="城市"></tiny-grid-column>
<tiny-grid-column title="产品">
<template #default="{ row }">
<div>
<tiny-switch modelValue=""></tiny-switch></div></template
></tiny-grid-column>
<tiny-grid-column title="操作">
<template #default="">
<tiny-button
text="删除"
:icon="TinyIconDel"
@click="(...eventArgs) => emit(eventArgs, row)"
></tiny-button></template></tiny-grid-column
></tiny-grid>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix template syntax errors in tiny-grid-column components

There are syntax errors in your template tags which may cause rendering issues:

  1. Missing closing angle brackets > in <template> tags.
  2. Incorrect closing of components.

Apply this diff to correct the syntax errors:

          <tiny-grid :fetchData="{ api: getTableData }">
            <tiny-grid-column type="index" :width="60" title=""></tiny-grid-column>
            <tiny-grid-column type="selection" :width="60"></tiny-grid-column>
            <tiny-grid-column field="employees" title="员工数">
-             <template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template
+             <template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template>
            </tiny-grid-column>
            <tiny-grid-column field="city" title="城市"></tiny-grid-column>
            <tiny-grid-column title="产品">
-             <template #default="{ row }">
+             <template #default="{ row }">
                <div>
                  <tiny-switch modelValue=""></tiny-switch>
-               </div></template
+               </div>
+             </template>
            </tiny-grid-column>
            <tiny-grid-column title="操作">
-             <template #default="">
+             <template #default>
                <tiny-button
                  text="删除"
                  :icon="TinyIconDel"
                  @click="(...eventArgs) => emit(eventArgs, row)"
-               ></tiny-button></template></tiny-grid-column
+               ></tiny-button>
+             </template>
+           </tiny-grid-column>
          </tiny-grid>
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<tiny-grid :fetchData="{ api: getTableData }">
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column>
<tiny-grid-column type="selection" :width="60"></tiny-grid-column>
<tiny-grid-column field="employees" title="员工数">
<template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template
></tiny-grid-column>
<tiny-grid-column field="city" title="城市"></tiny-grid-column>
<tiny-grid-column title="产品">
<template #default="{ row }">
<div>
<tiny-switch modelValue=""></tiny-switch></div></template
></tiny-grid-column>
<tiny-grid-column title="操作">
<template #default="">
<tiny-button
text="删除"
:icon="TinyIconDel"
@click="(...eventArgs) => emit(eventArgs, row)"
></tiny-button></template></tiny-grid-column
></tiny-grid>
<tiny-grid :fetchData="{ api: getTableData }">
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column>
<tiny-grid-column type="selection" :width="60"></tiny-grid-column>
<tiny-grid-column field="employees" title="员工数">
<template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template>
</tiny-grid-column>
<tiny-grid-column field="city" title="城市"></tiny-grid-column>
<tiny-grid-column title="产品">
<template #default="{ row }">
<div>
<tiny-switch modelValue=""></tiny-switch>
</div>
</template>
</tiny-grid-column>
<tiny-grid-column title="操作">
<template #default>
<tiny-button
text="删除"
:icon="TinyIconDel"
@click="(...eventArgs) => emit(eventArgs, row)"
></tiny-button>
</template>
</tiny-grid-column>
</tiny-grid>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor-main refactor/develop branch feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant