Skip to content

Conversation

@ioedeveloper
Copy link
Collaborator

Fixes #950

@ioedeveloper ioedeveloper added remix-ide performance Related to performance improvement labels Apr 25, 2021
@ioedeveloper ioedeveloper force-pushed the optimise-FE branch 3 times, most recently from b5ad8f2 to 825e956 Compare April 29, 2021 11:11
.perform(() => {
browser
.testConstantFunction(addressRef, 'get - call', null, '0:\nuint256: 45')
.testConstantFunction(addressRef, 'getInt - call', null, '0:\nuint256: 45')
Copy link
Contributor

Choose a reason for hiding this comment

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

calling get should have worked.
getInt is calling the lib, but here we are testing that the lib is correctly linked to the contract.
the contract is:

function get () public view returns (uint) {
          return lib.getInt();
      }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@yann300 I will need your help to figure out why testing get from libs fail.

}

exists (path, cb) {
async exists (path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to add async if the content of the function is not using await ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding async returns a promise. I added it because fileManager expects a promise just like how remixdProvider returns a promise.

path = path.replace(/^\/|\/$/g, '') // remove first and last slash
if (path.startsWith(this.workspacesPath + '/' + this.workspace)) return path
if (path.startsWith(this.workspace)) return this.workspacesPath + '/' + this.workspace
if (path.startsWith(this.workspace)) return path.replace(this.workspace, this.workspacesPath + '/' + this.workspace)
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't understand this change, is this a fix for when a folder has the same name as the workspace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fixes an issue i had when querying provider.exists(). If i have a path like default_workspace/contracts/ballot.sol, before resolving if the path exists removePrefix() is called. Now with the previous check, since the path starts with default_workspace, .workspaces/default_workspace is returned and when this (.workspaces/default_workspace) is checked if it exists, it always returns true. My change fixes that.

createNonClashingName (name, fileProvider, cb) {
this.createNonClashingNameWithPrefix(name, fileProvider, '', cb)
},
async checkNonClashingNameAsync (name, fileManager, prefix = '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be createNonClashingNameAsync ?

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 can rename it to createNonClashingNameAsync. It is the same implementation but async without callbacks.

@bunsenstraat
Copy link
Collaborator

Cannot create workspaces, it actually creates a workspace directory, empty, in the default workspace. Somehow it also creates workspaces, sometimes, but you only see the list of spaces created before you create the new one. However it doesn't happen always, but it happens. Tried in incognito, cleared the storage. It also throws a lot of errors. The selector is stuck at connecting to localhost and can't find .workspaces. Who said coding wasn't fun?

Screenshot 2021-05-06 at 13 08 30

Screenshot 2021-05-06 at 13 13 01

Screenshot 2021-05-06 at 13 14 43

Screenshot 2021-05-06 at 13 14 33

@ioedeveloper ioedeveloper force-pushed the optimise-FE branch 2 times, most recently from cb53bdf to 9daf546 Compare May 12, 2021 12:50
@ioedeveloper ioedeveloper requested a review from yann300 May 12, 2021 12:50
@ioedeveloper ioedeveloper merged commit b7b54a7 into master May 12, 2021
@ioedeveloper ioedeveloper deleted the optimise-FE branch May 12, 2021 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autorebase performance Related to performance improvement remix-ide

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File Explorer rerendering the entire tree.

4 participants