Skip to content

Add many console methods#55

Merged
therustmonk merged 2 commits into
yewstack:masterfrom
rivertam:master
Dec 29, 2017
Merged

Add many console methods#55
therustmonk merged 2 commits into
yewstack:masterfrom
rivertam:master

Conversation

@rivertam

@rivertam rivertam commented Dec 29, 2017

Copy link
Copy Markdown
Contributor

Addresses #44

Also refactored the way ConsoleService works. It now creates an instance of Console, which itself can do (almost) everything the JS console can do.

I had to name some functions {function}_named or similar because Rust doesn't have function arity overloading, which I did not realize until today!

I find this methodology to be a bit more ergonomic than the currently implemented one. Definitely willing to hear criticism, though. I'm very new to Rust. =)

@rivertam

Copy link
Copy Markdown
Contributor Author

I also recognize that @deniskolodin is going to rewrite how services work. This was mostly just for Rust practice/obsession reasons.

@rivertam

Copy link
Copy Markdown
Contributor Author

I'm considering the dependency injection implications of this... If we wanted to inject a different type of console service onto context, it would still have to return a Console struct. I'm not sure that's ideal, because then the injector can't override the side effects of the console. In C++, I'd use an interface called Console with a default implementation and have the default ConsoleService return a (smart?) pointer to the service, but I'm not sure how to do that using traits. Would it be by using Box?

@therustmonk

Copy link
Copy Markdown
Member

Hm... it looks pretty! 👍 Thank you!

I've fixed this:

warning: unused variable: `name`
  --> src/services/console.rs:12:25
   |
12 |     pub fn count(&self, name: &str) { js! { console.count(); } }
   |                         ^^^^
   |
   = note: #[warn(unused_variables)] on by default
   = note: to avoid this warning, consider using `_name` instead

And I've removed trace call from demo. It's cool, but it can scare novices )

@therustmonk
therustmonk merged commit 3a2ec6b into yewstack:master Dec 29, 2017
@therustmonk

Copy link
Copy Markdown
Member

With Rust you can add as many traits as you want. There are no conflicts in the future.
We also can use Console directly, without attaching it to the context, because it don't send messages back to the update loop. I want to let this API to be little experimental and we can compare and fix specific approach later.

@rivertam

rivertam commented Dec 29, 2017

Copy link
Copy Markdown
Contributor Author

Thanks for the catch! My bad. Agreed with .trace. Just wanted to test it.

I'm not sure what you mean with your second comment. I'm thinking of the testing story, in which we might want to swap out services with spies, mocks, or stubs. If we expect a specific Console struct to be returned, it must have that implementation because it's not "virtualized". One could not replace it with a struct with the same API but different behavior.

Truth be told, I have no idea how dependency injection would work with Yew at all, or even Rust for that matter. It would be nice to have it available, though. When I think "services", I do think dependency injection. But I think to do that, we'd need to expose services as a kind of middleware that dynamically transforms context rather than traits that statically add to it. But I'm pretty out of my element at this point.

teymour-aldridge referenced this pull request in teymour-aldridge/yew Jul 8, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
jstarry pushed a commit to jstarry/yew that referenced this pull request Jul 9, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
jstarry pushed a commit that referenced this pull request Jul 9, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
jstarry pushed a commit to jstarry/yew that referenced this pull request Jul 9, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
jstarry pushed a commit that referenced this pull request Jul 9, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
jstarry pushed a commit that referenced this pull request Aug 16, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
jstarry pushed a commit that referenced this pull request Aug 16, 2020
* Added demo doctest.

* Added github action.

* Move `test.yml` into correct folder.

* Add remaining code snippets.

* Fixed a syntax error in `test.yml`
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.

2 participants