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: reduce builder component bundle size [SPMVP-6056] #92

Merged
merged 7 commits into from
Aug 4, 2023

Conversation

arielllin
Copy link
Contributor

@arielllin arielllin commented Aug 2, 2023

Jira

https://storipress-media.atlassian.net/browse/SPMVP-6056

🎩 Tophat

Do a thorough 🎩. What is tophatting?

Consider testing:

  • Existing functionality which could break due to your changes (e.g. global changes)
  • New functionality being introduced. Ensure it meets intended behavior
  • All different permutations (flows, states, conditions, etc) that are possible
  • If you are modifying something which is used in multiple places, 🎩 all affected areas not just what you intend to change

🎩 Instructions

Run yarn build in builder-component
Check:

  • 打包後 dist 下檔案正確 ⭕
  • style.css 有在 dist 下 ⭕
  • package.json exports 路徑正確 ⭕

Test:

  1. 複製 dist 到有使用 builder-component 專案底下 node_module
  2. Run yarn install
  3. Run yarn dev
  4. Play around to confirm no error and builder-component works normally ⭕

Related PRs

Checklist before requesting review

  • I have done a self-review of my own code (comment on code is not required but recommended)
  • I did the Tophat steps and confirm the issue fixed
  • I have confirmed there is no issue reported by Static Analyzer (Please double check for custom class. For other issues, if you think it's ignorable, please add // eslint-ignore-next-line <rule name> on it)
  • I have confirmed there is no deepsource issue (if you think it's ignorable, please explain why and add a skipcq comment)
  • I confirmed that the unit test is passed (if you break it, please fix it in this PR)
  • I confirmed that I didn't break E2E test (if you break it, please fix it or create a new Jira)

Emoji Guide

For reviewers: Emojis can be added to comments to call out blocking versus non-blocking feedback.

E.g: Praise, minor suggestions, or clarifying questions that don’t block merging the PR.

🟢 Nice refactor!

🟡 Why was the default value removed?

E.g: Blocking feedback must be addressed before merging.

🔴 This change will break something important

Blocking 🔴 ❌ 🚨 RED
Non-blocking 🟡 💡 🤔 💭 Yellow, thinking, etc
Praise 🟢 💚 😍 👍 🙌 Green, hearts, positive emojis, etc

Gif (optional)

image

@arielllin arielllin requested a review from DanSnow August 2, 2023 07:11
plugins: [
baseConfig.plugins,

Vue({
Copy link
Contributor

Choose a reason for hiding this comment

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

這邊把整個 vue 的 plugin 移除會導致 vite 無法處理 vue 的檔案喔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

@arielllin arielllin requested a review from DanSnow August 4, 2023 01:45
@DanSnow DanSnow merged commit fe2f8e9 into master Aug 4, 2023
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.

2 participants