fix(docs): Remove environmental configuration and add user configuration prompts #3109
Conversation
WalkthroughThe changes involve the removal of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (4)
🪧 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 (
|
WalkthroughThis pull request updates the documentation by removing the environmental configuration from Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/sites/demos/pc/webdoc/develop-demo-en.md(1 hunks)examples/sites/demos/pc/webdoc/develop-demo.md(1 hunks)examples/sites/src/views/docs/docs.vue(3 hunks)
🔇 Additional comments (5)
examples/sites/demos/pc/webdoc/develop-demo.md (1)
51-52: Security improvement in Vite configurationThe removal of the
define: { 'process.env': { ...process.env } }configuration is a positive security change. This prevents environment variables from being exposed in client-side code, which could potentially leak sensitive information.examples/sites/demos/pc/webdoc/develop-demo-en.md (1)
51-52: Security improvement in Vite configurationThe removal of the environment variables configuration from the English documentation matches the change in the Chinese version, ensuring consistency across documentation while enhancing security.
examples/sites/src/views/docs/docs.vue (3)
3-21: Well-implemented user notification modalThe modal clearly explains the security risks of the old configuration and provides actionable solutions (upgrading to version 3.13.0+ or modifying the configuration). The UI is user-friendly with appropriate controls.
33-33: Appropriate component importsThe imports for TinyModal and TinyButton components are correctly added from the @opentiny/vue library.
95-97: Appropriate styling for modal keywordsThe styling for keywords in the modal enhances readability and helps users identify important information.
chenxi-20
force-pushed
the
fix/docs-20250311
branch
from
41cd177 to
d340d94
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/sites/src/views/docs/docs.vue (2)
3-21: Consider internationalization for modal content.The modal content is currently only in Chinese, which may not be accessible to all users of the library. Since this is providing important security information, consider supporting internationalization.
You could use your existing translation mechanism to support multiple languages:
<tiny-modal v-model="showModal" title="请注意" show-footer> <div class="modal-body"> - TinyVue 从 <span class="modal-body-keyword">3.13.0</span> 开始不需要在 - <span class="modal-body-keyword">vite.config.js</span> 文件中配置 - <span class="modal-body-keyword">define: { 'process.env': { ...process.env } }</span> - 这段代码，这段代码会导致环境变量被打包到构建产物中，引起信息安全风险，请业务尽快升级到 - <span class="modal-body-keyword">3.13.0</span> 或以上版本！如果不升级版本可以改成：<span - class="modal-body-keyword" - >define: { 'process.env': { }}</span - > - 同样也可以解决此问题！感谢您对 TinyVue 支持！ + {{ $t('securityNotice.message', { + version: '3.13.0', + configFile: 'vite.config.js', + configCode: 'define: { \'process.env\': { ...process.env } }', + alternativeCode: 'define: { \'process.env\': { } }' + }) }} </div> <template #footer> <tiny-button type="primary" :disabled="disabled" @click="handleConfirm">{{ - disabled ? `${time}S后可关闭此提示` : '确认将不再弹出此提示' + disabled ? $t('securityNotice.countdown', { time }) : $t('securityNotice.confirm') }}</tiny-button> - <tiny-button @click="handleCancel">取消</tiny-button> + <tiny-button @click="handleCancel">{{ $t('securityNotice.cancel') }}</tiny-button> </template> </tiny-modal>
38-38: Add error handling for localStorage operations.The code assumes localStorage is always available and accessible, but there are scenarios where it might be disabled or throw exceptions (private browsing mode, storage quotas, etc.).
Consider adding try/catch blocks:
-const tipFlag = localStorage.getItem('tiny-vue-env-tip') +let tipFlag = null; +try { + tipFlag = localStorage.getItem('tiny-vue-env-tip') +} catch (e) { + console.warn('Unable to access localStorage:', e) +} // Later in handleConfirm const handleConfirm = () => { showModal.value = false - localStorage.setItem('tiny-vue-env-tip', 'never') + try { + localStorage.setItem('tiny-vue-env-tip', 'never') + } catch (e) { + console.warn('Unable to save preference to localStorage:', e) + } }Also applies to: 55-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/sites/demos/pc/webdoc/develop-demo-en.md(1 hunks)examples/sites/demos/pc/webdoc/develop-demo.md(1 hunks)examples/sites/src/views/docs/docs.vue(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/sites/demos/pc/webdoc/develop-demo.md
- examples/sites/demos/pc/webdoc/develop-demo-en.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Unit Test
🔇 Additional comments (1)
examples/sites/src/views/docs/docs.vue (1)
42-52: Timer needs cleanup to prevent memory leaks.The timer is initialized when
showModalis true, but there's no cleanup mechanism if the component is unmounted before the timer completes, which could lead to memory leaks.Consider using
onBeforeUnmountto clean up the timer:import { ref, nextTick, watch, onMounted, shallowRef } from 'vue' +import { ref, nextTick, watch, onMounted, onBeforeUnmount, shallowRef } from 'vue' const tipFlag = localStorage.getItem('tiny-vue-env-tip') const showModal = ref(tipFlag !== 'never') const disabled = ref(true) let time = ref(5) +let timer = null if (showModal.value) { - let timer = setInterval(() => { + timer = setInterval(() => { if (time.value > 0) { time.value = time.value - 1 } else { disabled.value = false clearInterval(timer) timer = null } }, 1000) } + +onBeforeUnmount(() => { + if (timer) { + clearInterval(timer) + timer = null + } +})
chenxi-20
force-pushed
the
fix/docs-20250311
branch
from
d340d94 to
71955bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
examples/sites/src/views/docs/docs.vue (1)
43-52: 🛠️ Refactor suggestionMissing timer cleanup on component unmount
The timer is created when showModal is true, but there's no cleanup mechanism if the component is unmounted before the timer completes, which could lead to memory leaks.
Consider adding cleanup in an onBeforeUnmount hook:
-import { ref, nextTick, watch, onMounted, shallowRef } from 'vue' +import { ref, nextTick, watch, onMounted, onBeforeUnmount, shallowRef } from 'vue' const tipFlag = localStorage.getItem('tiny-vue-env-tip') const showModal = ref(tipFlag !== 'never') const disabled = ref(true) let time = ref(5) +let timer = null if (showModal.value) { - let timer = setInterval(() => { + timer = setInterval(() => { if (time.value > 0) { time.value = time.value - 1 } else { disabled.value = false clearInterval(timer) timer = null } }, 1000) } + +onBeforeUnmount(() => { + if (timer) { + clearInterval(timer) + timer = null + } +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/sites/demos/pc/webdoc/develop-demo-en.md(1 hunks)examples/sites/demos/pc/webdoc/develop-demo.md(1 hunks)examples/sites/demos/pc/webdoc/installation-en.md(1 hunks)examples/sites/demos/pc/webdoc/installation.md(1 hunks)examples/sites/src/views/docs/docs.vue(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/sites/demos/pc/webdoc/develop-demo-en.md
- examples/sites/demos/pc/webdoc/develop-demo.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Unit Test
🔇 Additional comments (3)
examples/sites/src/views/docs/docs.vue (3)
3-21: Modal implementation looks goodThe modal effectively warns users about the security risks of including environment variables in their builds and provides clear instructions on how to update their configuration.
38-56: Storing user preferences in localStorageStoring user preferences in
localStoragecan be a security risk if sensitive information is stored. Ensure that the information stored is not sensitive and consider using more secure storage methods if necessary.
99-101: CSS styling for keywords looks goodThe styling for the modal keywords effectively highlights important information.
| define: { | ||
| 'process.env': { ...process.env } | ||
| } | ||
| plugins: [vue()] |
There was a problem hiding this comment.
The Vite configuration has been simplified, but without updating the documentation
The code example has been simplified to remove the define property, but the tip in line 50 still references configuring TINY_MODE in process.env, which is no longer shown in the example. This creates a disconnect between the instructions and the provided code example.
Consider updating the tip to explain how to set environment variables with the new simplified configuration or add the proper configuration example:
plugins: [vue()]
+ // If you need to set environment variables:
+ define: {
+ 'process.env': {
+ TINY_MODE: 'pc'
+ }
+ }📝 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.
| plugins: [vue()] | |
| plugins: [vue()] | |
| // If you need to set environment variables: | |
| define: { | |
| 'process.env': { | |
| TINY_MODE: 'pc' | |
| } | |
| } |
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
Summary by CodeRabbit
New Features
Refactor
Style