Skip to content

feat: [select]reduce common package size #2577

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 29, 2024
Merged

feat: [select]reduce common package size #2577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internals/cli/src/config/vite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const getAlias = (vueVersion: string | number, theme = '', design) => {
'@opentiny/vue-theme-mobile': pathFromWorkspaceRoot('packages/theme-mobile/src'),
'@opentiny/vue-theme': pathFromWorkspaceRoot(`packages/theme${ns(design || theme)}/src`),
'@opentiny/vue-theme-saas': pathFromWorkspaceRoot('packages/theme-saas/src'),
'@opentiny/vue-common': pathFromWorkspaceRoot('packages/vue-common/src'),
'@opentiny/vue-common': pathFromWorkspaceRoot('packages/vue-common/src/index-dev'),
'@opentiny/vue-locale': pathFromWorkspaceRoot('packages/vue-locale/src'),

// 虚拟映射
Expand Down
3 changes: 1 addition & 2 deletions packages/vue-common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,13 @@
"bugs": {
"url": "https://github.com/opentiny/tiny-vue/issues"
},
"main": "src/index.ts",
"main": "src/index-dev.ts",
"sideEffects": false,
"type": "module",
"dependencies": {
"@opentiny/vue-locale": "workspace:~",
"@opentiny/vue-renderless": "workspace:~",
"@opentiny/vue-theme": "workspace:~",
"@opentiny/vue-theme-mobile": "workspace:~",
"tailwind-merge": "^1.8.0"
},
"scripts": {
Expand Down
5 changes: 5 additions & 0 deletions packages/vue-common/src/index-dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { customDesignConfig } from './index'
import { twMerge } from 'tailwind-merge'
export * from './index'
// 动态注入twMerge，保证本地联调
customDesignConfig.twMerge = twMerge
26 changes: 11 additions & 15 deletions packages/vue-common/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
} from './adapter'
import { t } from '@opentiny/vue-locale'
import { stringifyCssClass, stringifyCssClassObject, stringifyCssClassArray, deduplicateCssClass } from './csscls'
import { twMerge } from 'tailwind-merge'
import '@opentiny/vue-theme/base/index.less'
import { defineComponent, isVue2, isVue3 } from './adapter'
import { useBreakpoint } from './breakpoint'
Expand Down Expand Up @@ -136,8 +135,6 @@ export const $setup = ({ props, context, template, extend = {} }) => {
return renderComponent({ view, props, context, extend })
}

export const mergeClass = /* @__PURE__ */ (...cssClasses) => twMerge(stringifyCssClass(cssClasses))

// 提供给没有renderless层的组件使用（比如TinyVuePlus组件）
export const design = {
configKey: Symbol('designConfigKey'),
Expand All @@ -162,31 +159,32 @@ interface DesignConfig {

interface CustomDesignConfig {
designConfig: null | DesignConfig
twMerge: (str: string) => string
}

// 允许自定义主题规范，适用于MetaERP项目
export const customDesignConfig: CustomDesignConfig = {
designConfig: null
designConfig: null,
twMerge: () => ''
}

export const mergeClass = (...cssClasses) => customDesignConfig.twMerge(stringifyCssClass(cssClasses))

export const setup = ({ props, context, renderless, api, extendOptions = {}, mono = false, classes = {} }) => {
const render = typeof props.tiny_renderless === 'function' ? props.tiny_renderless : renderless

// 获取组件级配置和全局配置（inject需要带有默认值，否则控制台会报警告）
const globalDesignConfig: DesignConfig = customDesignConfig.designConfig || hooks.inject(design.configKey, {})
const designConfig = globalDesignConfig?.components?.[getComponentName().replace($prefix, '')]

const specifyPc = typeof process === 'object' ? process.env?.TINY_MODE : null
const utils = {
$prefix,
t,
...tools(context, resolveMode(props, context)),
designConfig,
globalDesignConfig,
useBreakpoint
}
if (specifyPc !== 'pc') {
utils.mergeClass = mergeClass
useBreakpoint,
mergeClass
}

utils.vm.theme = resolveTheme(props, context)
Expand All @@ -205,11 +203,10 @@ export const setup = ({ props, context, renderless, api, extendOptions = {}, mon
a: filterAttrs,
d: utils.defineInstanceProperties,
dp: utils.defineParentInstanceProperties,
gcls: (key) => getElementCssClass(classes, key)
}
if (specifyPc !== 'pc') {
attrs.m = mergeClass
gcls: (key) => getElementCssClass(classes, key),
m: mergeClass
}

/**
* 修复 render 函数下 this.slots 不会动态更新的问题（vue3 环境没有问题）
* 解决方法：在 instance 下注入 slots、scopedSlots
Expand Down Expand Up @@ -271,8 +268,7 @@ export function svg({ name = 'Icon', component }) {
const attrs = isVue3 ? tinyTag : { attrs: tinyTag }
let className = 'tiny-svg'

const specifyPc = typeof process === 'object' ? process.env?.TINY_MODE : null
if (specifyPc !== 'pc' && isMobileFirst) {
if (isMobileFirst) {
className = mergeClass('h-4 w-4 inline-block', customClass || '', mergeProps.class || '')
}

Expand Down
3 changes: 2 additions & 1 deletion packages/vue-saas-common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"type": "module",
"dependencies": {
"@opentiny/vue-common": "workspace:~",
"@opentiny/vue-design-saas": "workspace:~"
"@opentiny/vue-design-saas": "workspace:~",
"tailwind-merge": "^1.8.0"
Copy link

@coderabbitai coderabbitai bot Nov 29, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Update tailwind-merge to the latest version 2.5.5

The current version (1.8.0) is significantly outdated compared to the latest version (2.5.5). While no security vulnerabilities were found, it's recommended to update to benefit from the latest improvements and bug fixes.

  • Update the dependency in packages/vue-saas-common/package.json: 
    "tailwind-merge": "^2.5.5"
🔗 Analysis chain

Verify tailwind-merge version and security

The package uses tailwind-merge@^1.8.0. Let's ensure this version is secure and up-to-date.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for latest version and security advisories
curl -s https://registry.npmjs.org/tailwind-merge/latest | jq '.version'

# Check for known vulnerabilities
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: NPM, package: "tailwind-merge") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Length of output: 428

},
"scripts": {
"build": "pnpm -w build:ui $npm_package_name",
Expand Down
5 changes: 4 additions & 1 deletion packages/vue-saas-common/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { customDesignConfig } from '@opentiny/vue-common'

import { twMerge } from 'tailwind-merge'
Copy link

@coderabbitai coderabbitai bot Nov 29, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the gathered information, I can now provide a final response:

Reconsider the tailwind-merge implementation strategy

The analysis reveals several concerns:

  • tailwind-merge is not tree-shakeable and adds ~7KB gzipped to the bundle
  • It's being used across multiple framework implementations (Vue, React, OpenInula) for a common mergeClass utility
  • There are existing class merging utilities in the codebase (clsx, classNames) that could be leveraged

Recommendations:

  • Move the class merging logic to a shared internal utility using existing lighter alternatives like clsx
  • If tailwind-merge specific features are required, implement it as an optional peer dependency
  • Consider creating a minimal implementation that only includes the merge rules you actually need
🔗 Analysis chain

Consider impact on package size when adding tailwind-merge

Adding tailwind-merge as a dependency seems to contradict the PR's objective of reducing the common package size. While this utility is useful for merging Tailwind CSS classes, we should verify its impact on the final bundle size.

Let's analyze the impact:

Consider these alternatives to minimize bundle size:

  1. Lazy load tailwind-merge only when needed
  2. Create a minimal version with only required merge rules
  3. Consider moving this to an optional peer dependency if not widely used

Also applies to: 9-9

Let me gather more information about how twMerge is actually used in these files to better understand its necessity and implementation.

This will help us understand:

  1. How twMerge is being used across different packages
  2. If there are any existing class merging utilities that could be used instead
  3. Whether the implementation is consistent across different frameworks (Vue, React, OpenInula)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the size of tailwind-merge package
curl -s https://bundlephobia.com/api/size?package=tailwind-merge | jq '{
  size: .size,
  gzip: .gzip,
  treeshaking: .hasJSModule,
  dependencies: .dependencyCount
}'

# Check if twMerge is actually used in the codebase
rg -l "twMerge" --type ts

Length of output: 468

Script:

#!/bin/bash
# Check the actual usage of twMerge in the files
for file in $(rg -l "twMerge" --type ts); do
  echo "=== $file ==="
  rg "twMerge" -B 2 -A 2 "$file"
done

# Check if there are any existing class merging utilities
rg -l "classNames|clsx|cls|mergeClasses" --type ts

Length of output: 4759

import designSaasConfig from '@opentiny/vue-design-saas'

export * from '@opentiny/vue-common'
Copy link

@coderabbitai coderabbitai bot Nov 29, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Review re-export strategy

Re-exporting everything from @opentiny/vue-common might include unnecessary exports. Consider being more selective about what's re-exported to reduce the final bundle size.

Consider replacing the wildcard export with specific exports:

-export * from '@opentiny/vue-common'
+export { 
+  // List only the necessary exports
+  customDesignConfig,
+  // ... other required exports
+} from '@opentiny/vue-common'

Committable suggestion skipped: line range outside the PR's diff.


// 自定义主题交互规范
customDesignConfig.designConfig = designSaasConfig
customDesignConfig.twMerge = twMerge
Loading