-
Notifications
You must be signed in to change notification settings - Fork 3
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
building with vite #103
building with vite #103
Conversation
🦋 Changeset detectedLatest commit: a0e3744 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Great idea, looks good from what I can see.
Once it's released, I will try also to see if everything is still in order for the Trifid plugin :)
Big change is that the default package export is not UMD but ESM. This should work better, with code splitting. |
I added
I also made sure that the lock file includes entries for different architectures for rollup, so that we can build the app using different CPU architectures. |
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 did more deeper checks, and I think some changes could be great.
Once it's released, I will try also to see if everything is still in order for the Trifid plugin :)
Big change is that the default package export is not UMD but ESM. This should work better, with code splitting. Also, I'm unsure about PWA...
As I can see, it's exporting the UMD mostly like before: in dist/spex.umd.cjs
(it's now with .cjs
extension instead of .js
).
Also, dist/style.css
is the replacement for dist/spex.css
.
If we want to rename those files, should we consider a major version instead of a minor one? If not, can we make sure they get the same name as before?
For |
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.
Oh yes, you are right, we are still on 0.x.
So a breaking change is that paths are also changing as reported.
LGTM.
I mentioned the path changes in the changeset |
Replaced webpack with vite: