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: produce single bundle for runtime with multiple entrypoints #8504

Merged
merged 12 commits into from
Apr 18, 2023

Conversation

gtm-nayan
Copy link
Contributor

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

ref #8491
This should let us get rid of some manual import paths bookkeeping.

@vercel
Copy link

vercel bot commented Apr 15, 2023

@gtm-nayan is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Apr 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svelte-dev-2 ❌ Failed (Inspect) Apr 15, 2023 3:55pm

@benmccann
Copy link
Member

We were previously producing multiple bundles I think? If it's just one bundle now, do we need to update the exports in package.json?

@gtm-nayan
Copy link
Contributor Author

It's a single bundle in the sense that rollup considers all the files as part of the same bundle and links them as such. But, the output structure remains the same since there are multiple entry points now and output files defined as before for each of them.

The exports don't have to be changed.

Comment on lines 114 to 117
},
},
],
},
Copy link
Member

Choose a reason for hiding this comment

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

we've never had trailing commas before. I guess when we setup prettier it will probably remove them

Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have a prettier config in sites repo, we can just remove the source folders from prettierignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't introduce prettier just yet, the resulting diff would cause conflicts in a lot of the pulls targeted for v4.

gtm-nayan and others added 5 commits April 17, 2023 07:28
doesn't really make sense for a library since users'
build step will do it again anyway
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

This is better I'd say. It creates more than a single bundle for runtime/internal, there's also dev-[hash] and Component-[hash] files now, but AFAIK that doesn't have any negative implications.

@dummdidumm dummdidumm merged commit 662804e into sveltejs:version-4 Apr 18, 2023
dummdidumm pushed a commit that referenced this pull request Apr 18, 2023
…8504)

* single runtime bundle

* formatting

* dedupe output options

* fix tests apparently

* skip writeBundle for cjs build

* revert quotes

* remove manualChunks

* some node16 module resolution compliance

* disable minifyInternalExports (doesn't really make sense for a library since users'
build step will do it again anyway)
@gtm-nayan gtm-nayan mentioned this pull request Jun 17, 2023
5 tasks
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.

4 participants