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

api bump v-3.2.1 #1089

Merged
merged 11 commits into from
Jan 26, 2021
Merged

api bump v-3.2.1 #1089

merged 11 commits into from
Jan 26, 2021

Conversation

niklabh
Copy link
Contributor

@niklabh niklabh commented Dec 28, 2020

  • test auth server
  • test node watcher
  • test harvester
  • test front end
  • test chain db watcher

@Tbaut
Copy link
Collaborator

Tbaut commented Jan 13, 2021

don't hesitate to seek help and open an issue on https://github.com/polkadot-js/api/ if there's anything you can't solve.
I'll take a quick look now.

@Tbaut
Copy link
Collaborator

Tbaut commented Jan 13, 2021

What I did:

Please make sure to triple-check the changes I made with the id params, and the councilMembers array. These were normal lint issues that came up because of the bump.

edit: a dependency bump needs to be made regularly, and many are still lacking. Not to mention the other projects such as auth-server that I didn't touch.

@niklabh
Copy link
Contributor Author

niklabh commented Jan 13, 2021

Thankyou very much

@@ -49,7 +49,7 @@
"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn",
"semi": [2, "always"],
"simple-import-sort/sort": "error",
"simple-import-sort/imports": "error",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a heads-up.
I bumped simple-import-sort, which resulted in lint error something like "cannot find simple-import-sort/sort rules".
You will 100% get the same in the other projects when bumping it as well.

const { api, isApiReady } = useContext(ApiPromiseContext);
const { addresses } = useContext(UserDetailsContext);

councilQueryresult.data?.councils?.[0]?.members?.forEach( member => {currentCouncil.push(member?.address);});
councilQueryresult.data?.councils?.[0]?.members?.forEach( member => {
setCurrentCouncil([...currentCouncil, member?.address]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

A if could be needed here, I don't see why member would be undefined, but adding something like the following wouln't hurt:

Suggested change
setCurrentCouncil([...currentCouncil, member?.address]);
if (member)
setCurrentCouncil([...currentCouncil, member.address]);
}


import Post from '../../components/Post/Post';
import { useBountyPostAndCommentsQuery } from '../../generated/graphql';
import FilteredError from '../../ui-components/FilteredError';
import Loader from '../../ui-components/Loader';

export default () => {
const { id } = useParams();
const { query } = useRouter();
Copy link
Collaborator

@Tbaut Tbaut Jan 13, 2021

Choose a reason for hiding this comment

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

The reason I did all these changes is because TS was complaining that "id can't be found in type {}".
The obvious problem here is the typing that is wrong (and this comes from @types/react-router-dom). I kind of when around it by using a hook that was there, and type correctly what I received with as string.

Another, maybe better way would be to keep using the useParams<Record<string, string>>(). Up to you what you prefer.

@niklabh
Copy link
Contributor Author

niklabh commented Jan 17, 2021

There is memory leak/infinte loop in polkadot.js api or react-context.

This example can reproduce the issue

import React, {useContext, useEffect} from 'react';

import { ApiPromiseContext } from '@substrate/react-context';

function App() {
  const { api, isApiReady } = useContext(ApiPromiseContext);

  useEffect(() => {

		if (!isApiReady){
			return;
		}

    api.query.timestamp.now().then(console.log);

  }, [api, isApiReady]);

  return (
    <div>
      Memory leak
    </div>
  );
}

export default App;

Screenshot from 2021-01-18 02-23-48

@Tbaut
Copy link
Collaborator

Tbaut commented Jan 18, 2021

@substrate/react-context has been upgraded afair, can you try with the earlier version? This is what changed, and could very well be the origin of the problem paritytech/substrate-js-utils@7172989#diff-8b186436e62efe7d5dd561abd16d87eddfb98281d8cbf8aef2ab6f6b0aabab86

@niklabh niklabh merged commit ff91b35 into master Jan 26, 2021
@niklabh niklabh deleted the api-v-3.2.1 branch January 26, 2021 17:51
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