Skip to content

refactor: compose complicated string with renderString - #5872

Closed
UncleBill wants to merge 3 commits into
developfrom
fix/render-string
Closed

refactor: compose complicated string with renderString#5872
UncleBill wants to merge 3 commits into
developfrom
fix/render-string

Conversation

@UncleBill

Copy link
Copy Markdown
Contributor

No description provided.

@UncleBill

Copy link
Copy Markdown
Contributor Author

Should be merged after #5871

@UncleBill
UncleBill marked this pull request as draft March 14, 2022 11:06
@UncleBill
UncleBill force-pushed the feat/popup-share branch 3 times, most recently from 38ff96b to 2b77f50 Compare March 22, 2022 08:51
Base automatically changed from feat/popup-share to develop March 23, 2022 07:35
@UncleBill
UncleBill marked this pull request as ready for review March 23, 2022 07:38
@UncleBill

Copy link
Copy Markdown
Contributor Author

Might rewrite with i18n instead.

Comment thread packages/mask/src/plugins/Trader/SNSAdaptor/trader/Trader.tsx Outdated
: ExtraKeys<HEAD> | K | ExtraKeys<TAIL>
: never

export function renderString<T extends string>(template: T, data: Record<ExtraKeys<T>, string | number>) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggest, rename the function name to renderTemplateString

@guanbinrui guanbinrui Mar 23, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about this already existing solution _.template?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lodash template function use eval function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We might rewrite with i18n instead. I18n engine itself provides template rendering. And these places miss i18n indeed.

@github-actions

github-actions Bot commented Mar 23, 2022

Copy link
Copy Markdown
Contributor

@github-actions
github-actions Bot temporarily deployed to pull request March 23, 2022 07:47 Inactive
@github-actions
github-actions Bot temporarily deployed to pull request March 24, 2022 01:04 Inactive
@UncleBill

Copy link
Copy Markdown
Contributor Author

Use i18n instead. Closing

@UncleBill UncleBill closed this Mar 31, 2022
@UncleBill
UncleBill deleted the fix/render-string branch March 31, 2022 06:41
@github-actions
github-actions Bot temporarily deployed to pull request March 31, 2022 06:46 Inactive
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.

3 participants