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

only load chains lib scripts when needed #8599

Merged
merged 3 commits into from
Mar 16, 2021
Merged

Conversation

octavioamu
Copy link
Contributor

Description

This pr move the chains js dependencies to an object and add a computed prop to with the tenants presents on the cart. For each tenant present on the cartData (grants) this will inject the proper chain js lib needed avoiding load unused big js libs if not going to be used.

For future chains additions we will need to just add the tenant name and the object with all the js endpoints (external or internal) following the same pattern we do for the tabs.

There is a special rule for kuzama as this one have diff chain id / name but is actually part of polkadot and use the same files.

With this approach cart seems to be loading normally again and even if you pick a polkadot grant then the load time doesn't fill tedious as the other js are not loaded (unless you pick one grant of each of our supported chains, then will be the same time as today online)

Refers/Fixes
Testing

@octavioamu octavioamu changed the title only load chains script if needed only load chains lib scripts when needed Mar 15, 2021
@owocki
Copy link
Contributor

owocki commented Mar 15, 2021

YESSSS!

have you tested eth/zksync checkout with this. everything still works?

@octavioamu
Copy link
Contributor Author

YESSSS!

have you tested eth/zksync checkout with this. everything still works?

Tested only eth for now, shouldn't be a problem for zksync as didn't changed nothing there but still want to test other chains on stage to be 100% sure there are no regressions.

Copy link
Contributor

@gdixon gdixon left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍 I've tested on staging with ETH, DOT, KSM and RSK - everything connects as it should and I've successfully made a contribution with DAI 🙌

Copy link
Contributor

@chibie chibie left a comment

Choose a reason for hiding this comment

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

awesome! much needed!

@thelostone-mc thelostone-mc changed the base branch from stable to master March 16, 2021 11:47
@thelostone-mc thelostone-mc changed the base branch from master to stable March 16, 2021 11:53
@octavioamu octavioamu merged commit e804f1d into stable Mar 16, 2021
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.

5 participants