-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[rfw] Add an example of using JavaScript with RFW #9110
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.
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
Conversation
I believe the |
@moluopro Please do not comment in completely unrelated PRs to request reviews. The proper escalation path, as explained in our contributor documentation, is to contact the reviewer via Discord.
The reviewer is @Hixie, the code owner for this package, not me. |
I’m really sorry! I saw in that PR that, since Hixie didn’t respond, he reached out to you, so I thought I could do the same. I’m not very familiar with this process, as this is my first time submitting a PR for this library. I apologize again for the inconvenience caused. |
It's an interesting example, but I don't really understand the use case for using RFW with a JS backend. If you're using JS, why not just send HTML? |
The main reason I want to submit this change is that the WASM example is not running. I believe it's important to ensure that the official examples work on all Flutter platforms. Secondly, JavaScript is a popular language with many users, so I chose it as the case. |
I think we should separate those two problems. Making the Wasm example work again would be good. But that's an entirely separate problem from using RFW from JS. |
Yes, you're right. Based on what I mentioned earlier: First, the wasm example is not working, so I think we should consider whether to remove it. Second, the JS example provided works across all platforms, so I think we could consider adding it (of course, depending on your opinion). Third, I’d like to know what you think we should do. So far, I’ve only seen the questions, and I’m not clear on your opinion.
|
As far as the Wasm example goes, if it doesn't work, we should fix it. Is there an issue filed about this problem? As far as the JS example goes, I would need to better understand why someone would want to use RFW with JS on the browser. It seems like a really weird combination of technologies. When would you want to use RFW in this way? |
The package that wasm depends on is no longer maintained, so I would like to use a popular programming language as an alternative. My idea is that simple. As for the fix to the original case you mentioned, maybe you can continue maintaining the wasm package. Thank you for your response, I now understand that you have no intention of merging this PR. I will close it later. |
Thank you again for your response! |
I misunderstood something about this patch -- I thought this was just JS in the browser, but it's using a JS runtime accessed by FFI. I do think that could be interesting. We'd need to make sure the tests all pass. (Sorry, I got confused by the discussion of Wasm and JS and misunderstood what the example did. I think this PR should be considered independent of the issues with Wasm.) |
Yes I get that now. Sorry for being slow. Before we can land anything though we would need to make sure the tests pass. Currently four checks are failing. |
This is my first time handling this kind of process, so it might take me a bit more time. |
…ple/javascript/android/app
Two issues remain and I need your help:
|
Yes, please file an issue for any problems with the flutter/packages repository tooling.
Please provide more information on why this package is necessary and how the maintainability aspect will be addressed. It's not entirely clear to me what the value of the example is if it only works with a package that has almost no usage (per pub.dev stats). |
The JSF package provides a runtime environment for executing JavaScript scripts:
|
Please see the link to our policy in the CI failure error message you quote. This doesn't address any of the issues described in that policy.
That doesn't seem to address the concern that the example is only useful with your package; if no other packages are sufficient for the example to make sense with them, isn't it still fundamentally an example specific to your package? I think the next step here is to do this required step that was previously skipped:
and use that issue to explain in more detail what problem this PR is intended to solve, and have that discussion there. Creating an exception to our policy requires a clear and strong justification that the exception is needed to solve an important use case. |
I believe I was merely adding an example, not modifying the actual codebase. So introducing a new package might not have that much impact. But you're right — the community has its own rules. Since this PR has already taken more time than I expected, I will no longer pursue getting it merged. Thank you for your response. |
Examples are part of the codebase. We build them in CI on every commit, and failures close the tree.
In the short term, that's true. But in a year, if you have abandoned that package for whatever reason, and it no longer works with Flutter |
You can take a look at the conversation above. Maybe JSF might not be well-maintained in the future, but the package used in the original example has already been unmaintained for several years. |
I have added an example using JavaScript with RFW. This new example essentially replicates the functionality of the previous wasm example. Since the wasm package has been marked as
discontinued
and can no longer run directly on certain platforms, I included this new example as a replacement.The new example is compatible with all Flutter platforms.
Pre-Review Checklist
[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3