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

chore(generate-icon): Compile from local svg directory #765

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

thinkasany
Copy link
Collaborator

@thinkasany thinkasany commented Apr 7, 2024

变更:从npm获取文件变成从本地svg目录开始编译,方便后续添加

  1. 去掉prettier是因为格式化的话,每次更新都很大
    会有40+ 文件因为文件名比较长导致格式化和其他的不一样,另外500+是正常的。后续我再来个format的pr来处理。
image

Copy link

changeset-bot bot commented Apr 7, 2024

⚠️ No Changeset found

Latest commit: e5e239f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Apr 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ant-design-web3 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 7, 2024 2:06pm

Copy link

github-actions bot commented Apr 7, 2024

Preview is ready

@@ -11,3 +11,4 @@ packages/mysolidity/solc
.umi-test
.umi
.changeset
packages/icons/src/components
Copy link
Member

Choose a reason for hiding this comment

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

感觉不太好整个目录禁用 prettier,可以给不需要格式化的行加上 // prettier-ignore 阻止格式化

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

现在都是模版统一生成的,其实格式上也过得去

Copy link
Member

Choose a reason for hiding this comment

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

怕的就是之后会对 components 中的文件进行微调,然后格式没有统一格式化

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我感觉不太建议手动微调了,否则每次都会被脚本更新的时候重新改变了,真的很特定的情况下可以看看再怎么去兼容。这边遇到的主要问题就是部分组件名字太长了,格式化之后换行,和模版不一样会显示很多更改,就不太好统一了。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

或者有什么建议么,我暂时没有更好的方法去解决这个问题。 好像 "printWidth": 100, 是这个参数的格式化规则,但是感觉不太应该去影响到项目的其他地方

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

// prettier-ignore 主要是目前需要加上这个的有40+ 不需要的500+ emmm..

Copy link
Collaborator

Choose a reason for hiding this comment

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

有没有办法只在构建产物里生成 icon 对应的 react component,源码里不包含。这样就不存在 format/lint 问题了,而且看起来更简单清晰。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

本地只留svg么,打包才生成组件,但是可能不利于观察组件generate情况,留本地的话,开发者还可以观察生成差异的情况。 其实我觉得那个格式化问题不是很大,因为可能就是 单行变成了120个字符上下,整体结构都是和模版一样的,也不会真的长了很多,也许等真正我们需要特殊处理的时候,有场景了对应的解决方案应该也好出的。

Copy link
Collaborator

@jeasonstudio jeasonstudio left a comment

Choose a reason for hiding this comment

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

LGTM,改为静态读取之后,是不是就可以用原生node而不是ts-node了

@thinkasany
Copy link
Collaborator Author

LGTM,改为静态读取之后,是不是就可以用原生node而不是ts-node了

但是脚本用了ts文件诶,想要执行那个文件的话好像还是得用ts-node,就算是 console.log('hello world'), 似乎也得用ts-node,或者后面可以看看能不能优化一下。

@thinkasany thinkasany merged commit c18e143 into ant-design:main Apr 8, 2024
7 checks passed
@jeasonstudio
Copy link
Collaborator

LGTM,改为静态读取之后,是不是就可以用原生node而不是ts-node了

但是脚本用了ts文件诶,想要执行那个文件的话好像还是得用ts-node,就算是 console.log('hello world'), 似乎也得用ts-node,或者后面可以看看能不能优化一下。

对的,我是感觉用静态读取之后 ts 变得不是特别必要了,脚本中移除类型其实影响并不大,而且还可以少引入一个运行时,具体你可以再看看。

@thinkasany
Copy link
Collaborator Author

LGTM,改为静态读取之后,是不是就可以用原生node而不是ts-node了

但是脚本用了ts文件诶,想要执行那个文件的话好像还是得用ts-node,就算是 console.log('hello world'), 似乎也得用ts-node,或者后面可以看看能不能优化一下。

对的,我是感觉用静态读取之后 ts 变得不是特别必要了,脚本中移除类型其实影响并不大,而且还可以少引入一个运行时,具体你可以再看看。

其实和直接写js也差不多哈哈哈,只是现在基本都在拥抱ts,目前可以先用着,后面说不定还要叠加功能的时候,看看怎么优化更好,你们也可以发群里或者issue,看看具体要怎么实现,然后到时候我可以去认领一下。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants