Skip to content

Conversation

@nojaf
Copy link
Member

@nojaf nojaf commented Jun 13, 2025

@jfrolich I tried this after building locally, cargo build --release.
Then ../rescript/rewatch/target/release/rewatch in my own project, and this didn't get picked up. Any ideas why?

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jun 13, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7547

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7547

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7547

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7547

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7547

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7547

commit: 040dc5e

pub fn get_jsx_preserve_args(&self) -> Vec<String> {
match self.jsx.to_owned() {
Some(jsx) => match jsx.preserve {
Some(true) => vec!["-bs-jsx-preserve".to_string()],
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not any new options with -bs prefix (and get rid of the old ones eventually).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the BSC flag; it was already in place. Why didn't you speak up when it was introduced?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right. Must have overlooked it at that point.

@nojaf
Copy link
Member Author

nojaf commented Jun 19, 2025

@jfrolich just try the pkg-pr version of this PR and the setting is not getting picked up.
Any idea what I'm missing here?

@nojaf nojaf marked this pull request as ready for review June 23, 2025 08:18
@nojaf nojaf requested review from Bushuo and jfrolich June 23, 2025 08:19
@nojaf
Copy link
Member Author

nojaf commented Jun 23, 2025

I'm a bit confused why the other jsx compiler arguments are currently not passed down.
But that last commit makes it work for me.

Copy link
Collaborator

@Bushuo Bushuo left a comment

Choose a reason for hiding this comment

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

Code looks good.
I can't answer your question regarding the args though.

@nojaf
Copy link
Member Author

nojaf commented Jul 1, 2025

Hmm, I guess we can merge and revisit if passing the other jsx args turns out to be a bad thing.

@nojaf nojaf merged commit e21d7c2 into rescript-lang:master Jul 1, 2025
27 of 29 checks passed
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.

3 participants