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.

Sign up for GitHub

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

Remove outdated npm instruction. #1757

Closed
wants to merge 1 commit into from
Closed

Remove outdated npm instruction. #1757

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions cmd/soroban-cli/src/commands/contract/bindings/typescript.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl NetworkRunnable for Cmd {
}
}
std::fs::create_dir_all(&self.output_dir)?;
let p: Project = self.output_dir.clone().try_into()?;
let project: Project = self.output_dir.clone().try_into()?;
print.infoln(format!("Network: {}", network.network_passphrase));
let absolute_path = self.output_dir.canonicalize()?;
let file_name = absolute_path
Expand All @@ -134,18 +134,14 @@ impl NetworkRunnable for Cmd {
.to_str()
.ok_or_else(|| Error::NotUtf8(file_name.to_os_string()))?;
print.infoln(format!("Embedding contract address: {contract_address}"));
p.init(
project.init(
contract_name,
&contract_address.to_string(),
&network.rpc_url,
&network.network_passphrase,
&spec,
)?;
print.checkln("Generated!");
print.infoln(format!(
"Run \"npm install && npm run build\" in {:?} to build the JavaScript NPM package.",
self.output_dir
));
Comment on lines -145 to -148
Copy link
Member

Choose a reason for hiding this comment

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

We just added this instruction because it is relevant to the typescript bindings, which create an npm package, but the cli no longer installs and builds the package. That is now the job of the user. This command is independent of the frontend template feature that used to exist in the contract init command.

Copy link
Member Author

@fnando fnando Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this instruction won't work unless you follow whatever we had on the template; npm run build is not a standard task and can be something completely different depending on how you set up your project.

Let's say you have a hello smart contract project, and the dapp lives somewhere else, and you're using stellar contract bindings typescript --out-dir ../hello-dapp, then this also is misleading, because I'd run it on the smart contract dir, which is not a js project.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, i just got the message. It doesn't mention the current dir, it mentions the output dir, which has everything mentioned on the task. My bad! 🤦‍♂️

Ok(())
}
}
Expand Down
Loading