Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

Rust NPM Package #60

Closed
wants to merge 5 commits into from
Closed

Rust NPM Package #60

wants to merge 5 commits into from

Conversation

droogmic
Copy link
Collaborator

@droogmic droogmic commented Feb 5, 2022

@dabreegster
Copy link
Contributor

I wonder if following this template is overkill. New dependencies on Travis and Appveyor build systems? We don't need full automation for publishing to NPM every time we commit here; we don't want that even! To start, if we manually run a few commands to publish, that'd almost be preferable.

Also not sure we need a new crate to expose a JS interface. In another project, I just tacked on the tiny WASM interface at the end: https://github.com/zonebuilders/zonebuilder-rust/blob/a3070b9332227f9ebc074c6879448d050ea99887/src/lib.rs#L272

I may have some time this weekend to play around with all of this. I'm fine moving forward with something working, but I would like to eventually go back and simplify it

@droogmic
Copy link
Collaborator Author

droogmic commented Feb 9, 2022

I wonder if following this template is overkill. New dependencies on Travis and Appveyor build systems? We don't need full automation for publishing to NPM every time we commit here; we don't want that even! To start, if we manually run a few commands to publish, that'd almost be preferable.

Oh, I need to cleanup the template (draft). I wuold expect a simple Github Actions based deployment...

Also not sure we need a new crate to expose a JS interface. In another project, I just tacked on the tiny WASM interface at the end: https://github.com/zonebuilders/zonebuilder-rust/blob/a3070b9332227f9ebc074c6879448d050ea99887/src/lib.rs#L272

Agreed, but I don't want to bring any WASM stuff into the default osm2lanes crate.
So either tack it onto osm2lanes-web or make it a feature flag on osm2lanes? I like the sound of the second option, if we can do it in a way that is hidden from rust x86 clients.

@dabreegster
Copy link
Contributor

So either tack it onto osm2lanes-web or make it a feature flag on osm2lanes?

osm2lanes-web is all about rendering / Yew; it's an "end-user app" at the moment, so probably not there.

The feature flags should be pretty simple: https://github.com/zonebuilders/zonebuilder-rust/blob/a3070b9332227f9ebc074c6879448d050ea99887/Cargo.toml#L21

So ideally all we need to do is add that to osm2lanes crate, do wasm-pack build --release, then do a few things to take the generated directory and get it on NPM. I don't know how NPM works yet, so maybe it's more elaborate than that

@droogmic
Copy link
Collaborator Author

I ran into some problems where target + crate-type + features could not be configured correctly to do the right thing.

See open issues like: rust-lang/cargo#1197

I am leaning back again to making this a separate crate :/

Copy link
Contributor

@dabreegster dabreegster left a comment

Choose a reason for hiding this comment

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

This seems like what I'd expect to need. I should just run things myself and see, but I don't understand what the problem is with tying everything to --features=wasm. But a separate crate also is fine if that's needed


[features]
default = ["console_error_panic_hook"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, why's this in default? Related to trying to make features per compilation target work?

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