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

chore: use rolldown #371

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

chore: use rolldown #371

wants to merge 8 commits into from

Conversation

manuel3108
Copy link
Member

Previous work #11

In contrast to my last try a few weeks ago, the conversion seems to be nearly flawless. Since rolldown is nearing a 1.0 release, i thought we could reconsider our decision

We are planning a 1.0.0 beta release this month and certainly we would have a ecosystem CI to run before publishing any new version.
source: #11 (comment)

Apart from the watch mode, everything seems to be working flawlessly. As for the watch mode I either made a configuration mistake, or there is an upstream bug. Every time I start the watch mode I get the following output:

> [email protected] dev D:\dev\web\svelte-cli
> rolldown --watch --config

Waiting for changes...
Waiting for changes...
Waiting for changes...
Waiting for changes...
Waiting for changes...
Waiting for changes...

Modifying any of the related files does not trigger a rebuild though. Maybe someone can test this on a unix system to see if it's windows related. Surprisingly noone seems to be using watch mode in a public github repository: https://github.com/search?q=%22rolldown+-w%22+OR+%22rolldown+--watch%22+path%3A**%2Fpackage.json&type=code&ref=advsearch

Copy link

changeset-bot bot commented Dec 23, 2024

🦋 Changeset detected

Latest commit: 737a6d1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sv Patch

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

@benmccann
Copy link
Member

I get Waiting for changes... six times when running pnpm dev on Linux as well. I guess because there's six sub-projects (in export default).

I also don't see any changes happening when I modify a file on Linux. Or at least nothing printed to the screen. I modified a file to have syntax errors and the screen didn't change at all

@benmccann
Copy link
Member

I noticed this is outputting sourcemaps. Do we need those? The package would probably be a lot lighter without them. No idea if it might also speed up compile times

@manuel3108
Copy link
Member Author

manuel3108 commented Dec 24, 2024

I get Waiting for changes... six times when running pnpm dev on Linux as well. I guess because there's six sub-projects (in export default).

Thanks for the confirmation. Identified the root issue (having more than 5 projects breaks watch mode, we have 6) and thus opened an upstream issue: rolldown/rolldown#3214

I noticed this is outputting sourcemaps. Do we need those? The package would probably be a lot lighter without them. No idea if it might also speed up compile times

This is also happening with our last release and seems to be the cause for about 50% of our bundle size. Investigating (#373)

@benmccann
Copy link
Member

Awesome detective work. Personally I don't really use pnpm dev much, so it wouldn't be a blocker for me, but you guys might feel differently about that. Hopefully it won't be too long before they fix it though

@manuel3108
Copy link
Member Author

Update to the 1.0 beta that was recently released. This also solved the watch problem mentioned above.

@manuel3108 manuel3108 marked this pull request as ready for review December 27, 2024 07:12
@benmccann
Copy link
Member

Looks like there's a merge conflict

@benmccann
Copy link
Member

one thing I wondered is if we should add a changeset. normally I wouldn't for a build change, but this one's big enough it might be worth mentioning so that if there's any type of issue that arises it might be easier for people to identify

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