-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix: dynamic import of vite #24745
fix: dynamic import of vite #24745
Conversation
Thanks for the contribution! |
Changed Packages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! 👍
'@backstage/cli': patch | ||
--- | ||
|
||
Fixed the dynamic import of vite. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if there's any particular version of vite
where the behavior changes? Thinking it would be good if we could highlight that in the changeset. What version are you testing with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing with these versions
"@vitejs/plugin-react": "^4.0.4",
"vite": "^4.4.9",
"vite-plugin-html": "^3.2.0",
"vite-plugin-node-polyfills": "^0.21.0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which seems to match the backstage cli vite versions:
backstage/packages/cli/package.json
Lines 193 to 196 in 95ee6d6
"@vitejs/plugin-react": "^4.0.4", | |
"vite": "^4.4.9", | |
"vite-plugin-html": "^3.2.0", | |
"vite-plugin-node-polyfills": "^0.21.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which version is it actually resolved to though in yarn.lock
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
our own backstage instance:
└─ app@workspace:packages/app
└─ vite@npm:4.5.3 [f87a9] (via npm:^4.4.9 [f87a9])
while in my fork on this branch:
├─ @backstage/cli@workspace:packages/cli [05476]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [07278]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [11fe3]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [31634]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [39bca]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [70735]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [aad9f]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [b9c15]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ @backstage/cli@workspace:packages/cli [c856b]
│ └─ vite@npm:4.5.3 [23bd4] (via npm:^4.4.9 [23bd4])
│
├─ example-app-next@workspace:packages/app-next
│ └─ vite@npm:4.5.3 [d47fc] (via npm:^4.4.9 [d47fc])
│
└─ example-app@workspace:packages/app
└─ vite@npm:4.5.3 [d47fc] (via npm:^4.4.9 [d47fc])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should actually bite the bullet and bump to vite@v5
? Version 6 is on it's way soon, and from what I can tell I don't think that there's any breaking changes that we would run into.
c9e619f
to
64c8cef
Compare
I tried a bump to |
Signed-off-by: Alper Altay <[email protected]>
Signed-off-by: Alper Altay <[email protected]>
64c8cef
to
f8eeeb8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's 👍
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
Uffizzi Cluster |
Hey, I just made a Pull Request!
Fixes #24678
✔️ Checklist
Signed-off-by
line in the message. (more info)