Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

Bundle size too large due to dev users #1053

Closed
statictype opened this issue Jun 9, 2023 · 6 comments · Fixed by #1070
Closed

Bundle size too large due to dev users #1053

statictype opened this issue Jun 9, 2023 · 6 comments · Fixed by #1070
Assignees
Labels
bug Something is broken

Comments

@statictype
Copy link

statictype commented Jun 9, 2023

~3.5MB of the minified and gzipped production bundle size comes from dev users public keys artifact, without actually importing any dev users in the project. Looks like a regression of #951

The reasoning behind this approach of creating 100k users for testing is not clear to me.

Screenshot 2023-06-09 at 13 17 47
@statictype statictype added the bug Something is broken label Jun 9, 2023
@tjjfvi
Copy link
Contributor

tjjfvi commented Jun 13, 2023

The reasoning behind this approach of creating 100k users for testing is not clear to me.

All the test users we ultimately want to use need to be generated upfront as they are all put into the chain spec before launch. Since we cannot know how many test users we may need, we chose an arbitrarily high number, that should be enough for any usecase. However, it takes a while to generate all these users, especially in CI, so to improve performance, we generate this once and commit it.

This file is only ever needed on the server, though, so it should not be included in the client bundle. We'll tweak the dependency graph to make sure it's not included from the mod.ts.

@tjjfvi
Copy link
Contributor

tjjfvi commented Jun 13, 2023

On another note, you might be happy to hear that the second-largest rectangle there (capi_crypto_wrappers.generated.js) will be gone as of #1060

@statictype
Copy link
Author

i just wonder what was the issue you encountered which led you to conclude there is a need for many different users.
the default dev accounts seem to be sufficient for all testing needs i know of.
is restarting the dev chain not possible? or transferring smaller amounts to prevent running out of balance? set the initial balance of default dev accounts to max u128? sudo set balance before all tests?

@tjjfvi
Copy link
Contributor

tjjfvi commented Jun 13, 2023

If two different tests running in parallel use the same test users, they will attempt to send two extrinsics with the same nonce, which will result in one of the two failing.

@statictype
Copy link
Author

finding a way to increment the nonce sounds like a better way to go than generating 100k test users, imho
i am sure this is possible.

@harrysolovay
Copy link
Contributor

@statictype I understand your concern, but I believe the system for vending untouched test users is preferable. It allows us to forgo thinking about potential conflicts between integration tests. Hopefully your concern will be mitigated by some changes that @kratico is soon to PR, which will ensure the large dev users artifact isn't accidentally exposed through Capi's root.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something is broken
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants