Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Default Develop port to match metamask & ganache-cli default port #1579

Closed
wants to merge 3 commits into from

Conversation

CruzMolina
Copy link
Contributor

Per @adrianmcli's feedback

Defaults truffle develop to use Metamask & ganache-cli's default port (8545) when no network configuration is defined.

Should allow a user to unbox a drizzle box, call truffle develop, and connect using MM out of the box.

(Should pass tests and build successfully once v4 test leftovers are updated)

@CruzMolina CruzMolina changed the title Default Develop port to match metamask & ganache-cli Default Develop port to matc metamask & ganache-cli default port Dec 21, 2018
@CruzMolina CruzMolina changed the title Default Develop port to matc metamask & ganache-cli default port Default Develop port to match metamask & ganache-cli default port Dec 21, 2018
@adrianmcli
Copy link
Contributor

Gotta make sure that all the boxes are updated (config, docs, client, etc.) after this is merged.

@@ -83,7 +83,7 @@ var command = {

var ganacheOptions = {
host: "127.0.0.1",
port: 9545,
port: 8545,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this! If Ganache is already running, then truffle develop will fail to start, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be prudent to error out and let the user know something like "Looks like you already have Ganache running on port 8545. Either change the port or turn off Ganache and try the command again."

Copy link
Contributor

Choose a reason for hiding this comment

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

The whole point of truffle develop is that it's low friction. That's why we chose 9545 for truffle develop in the first place and 7545 for Ganache UI

Copy link
Contributor

Choose a reason for hiding this comment

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

@gnidan but you almost never have both running, and I've spoken with many dapp developers in the community that agree with me on this. You always end up having to change the port in your frontend or elsewhere. They should all be 8545 in my opinion and I'd fight for this one very strongly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gnidan in v5/next/develop the current behavior is that truffle develop doesn't clash (out of the box) with ganache-cli or ganacheUI.

on this branch/PR, truffle develop doesn't clash with ganacheUI out of the box, but if ganache-cli is already running and you try to spin up truffle develop, it'll clash and throw an error (because of the timestamped network_id's ganache-cli is given). However, if you spin up truffle develop and then start up ganache-cli, truffle will auto-connect to ganache-cli.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.626% when pulling b48df01 on fix-develop-port into 7ea11ab on develop.

@gnidan
Copy link
Contributor

gnidan commented Jan 2, 2019

(This is awaiting discussion / requirements gathering)

@CruzMolina
Copy link
Contributor Author

Closing this for now as #1773 seems to be the current reasonable compromise. 😎

@CruzMolina CruzMolina closed this Mar 19, 2019
@adrianmcli
Copy link
Contributor

adrianmcli commented Mar 19, 2019

@CruzMolina Cool to see this being addressed. Was just wondering if you don't mind outlining what would be the recommended path forward for teaching beginners then? Just want to make sure it's spelled out clearly so that there's no confusion on what to do.

@CruzMolina
Copy link
Contributor Author

@adrianmcli I just looked over the "Getting Started With Drizzle And React" tutorial.

There's a discrepancy w/ MM's UI (on 6.2.2 on Chrome).

Screen Shot 2019-03-19 at 9 37 37 AM

Currently you have to set port: 8545 in truffle-config.js before running truffle develop to get MM to connect to the local chain.

If their intent is to support localhost 8545 then the tutorial would need to be updated to include setting your config like so:

module.exports = {
    develop: {
      port: 8545
     // doesn't need other network configs
     // safe localhost default params are used
    }
  }
};

@CruzMolina
Copy link
Contributor Author

Opened an issue w/ MM regarding the above noted issue at: MetaMask/metamask-extension#6321

@gnidan gnidan deleted the fix-develop-port branch October 29, 2019 15:42
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.

4 participants