Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

manual build via rollup #109

Merged
merged 5 commits into from
Dec 25, 2020
Merged

manual build via rollup #109

merged 5 commits into from
Dec 25, 2020

Conversation

phryneas
Copy link
Collaborator

Putting this up here to see if CSB & co work. Still got some work to do.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 13, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 58a780e:

Sandbox Source
React Configuration
React Typescript Configuration
rtk-query-demo Configuration
svelte-app-rtk-simplequery-demo Configuration

@github-actions
Copy link

github-actions bot commented Dec 13, 2020

size-limit report 📦

Path Size
dist/rtk-query.cjs.production.min.js 0 B (-100% 🔽)
dist/rtk-query.esm.js 0 B (-100% 🔽)
ESM full 10.71 KB (+100% 🔺)
createPureApi + setupListeners 8.83 KB (+100% 🔺)
createApi + setupListeners 9.71 KB (+100% 🔺)
fetchBaseQuery 631 B (+100% 🔺)
retry 271 B (+100% 🔺)
ApiProvider 392 B (+100% 🔺)
CJS minfied 16.14 KB (+100% 🔺)

{
"name": "createApi + setupListeners",
"path": "dist/esm/index.js",
"import": "{ createApi, setupListeners }"
Copy link
Collaborator

@markerikson markerikson Dec 13, 2020

Choose a reason for hiding this comment

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

Oh wow, I had no idea this was a thing! That's really cool!

Can we set this up for RTK too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will, as soon as I recreate the changes I did here over there ;)

[
'@babel/preset-env',
{
targets: { node: true, browsers: ['defaults', 'not IE 11', 'maintained node versions'] },
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So this now has two builds: an ESM build that does not bundle up the transpiled files, so just a file-for-file transpilation. This should help modern bundlers with tree-shaking quite a lot.
The target I chose for that is "all non-dead browsers, last 2 versions, no IE, maintained node versions".

The CJS build includes IE as well and builds only one file.

Do you think that's sensible @markerikson ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For reference: including IE there bumps the bundle sizes for the ESM module up to

  ESM full
  Size limit: 15 KB
  Size:       13.8 KB with all dependencies, minified and gzipped
  
  createApi + setupListeners
  Size: 12.55 KB with all dependencies, minified and gzipped
  
  fetchBaseQuery
  Size: 3.23 KB with all dependencies, minified and gzipped
  
  retry
  Size: 2.79 KB with all dependencies, minified and gzipped
  
  ApiProvider
  Size: 387 B with all dependencies, minified and gzipped

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, fine by me.

rollup.config.js Outdated
@@ -74,6 +75,11 @@ const configs = [
? defaultTsConfig
: { ...defaultTsConfig, declarationDir: 'dist/ts', declaration: true, declarationMap: true }
),
replace({
values: {
'process.env.NODE_ENV': JSON.stringify(minfied ? 'production' : 'development'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo here? minfied

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yah, but just a variable name, will work either way 🤣

@msutkowski msutkowski marked this pull request as ready for review December 17, 2020 22:30
@phryneas
Copy link
Collaborator Author

I guess letting this sit around any longer won't do any good -merging it in :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants