openstreetmap
Add support for ReactJS
This PR adds ReactJS support (see discussion in #6540).
I converted modules/ui/feature_info.js into a React component as a demo. (The feature info is the blue box in the footer saying something like "30 hidden objects").
It shows:
- how to integrate a React component into D3 - see call of
render - how to integrate a D3 component into React - see method
refMain
I used the TypeScript compiler for translating JSX. I prefer TypeScript over Babel since it comes with less dependencies and less configuration. Further more it allows to use modern ES6 JavaScript (see function bindMany which will be transpiled into ES5.
Using the TypeScript compiler would obviously also allow to use TypeScript in the source code. I know, TypeScript for source was rejected by @bhousel in 2018. So please note: This would be possible, but is totally optional. This topic would certainly be worth a discussion. But since it's optional for this PR, discussing TypeScript for source code should take place in an extra issue.
Further more I added npm run watch:dev which runs the JavaScript compilation in watch mode.
@til-schneider, is this PR inactive? Or could we ask a maintainer to merge it?
Oh, I would love to see this PR being merged. However there was no reaction on it yet. I would have to rebase the branch and update the build so it can be merged with the current main branch. But before I do this effort I would like to get a comment from the maintainers whether they would merge this PR in the first place.
@mbrzakovic, this PR was made to solve the issue #6540, opened by @quincylvania. Could you approve this PR to simply an onboarding of new contributors?
For what it's worth, we have merged some example code into RapiD a few months ago that allows us to write components with ReactJS, but we're not using it yet.
If you have a fork of iD where you're building UI elements in ReactJS, consider forking off the RapiD code instead, and using this approach to create containers for ReactDOM to render (you can even use JSX or TSX if you want): https://github.com/facebookincubator/RapiD/commit/034c3316d26455a462fad876a303cf175475ccf4
@bhousel , could you please share the documentation or description where I could find how features comes from RapID to ID?
could you please share the documentation or description where I could find how features comes from RapID to ID?
I don’t have any documentation like that, sorry..
If I understand @bhousel correctly, he won't merge this PR since he would prefer the solution he build in RapiD. @bhousel please correct me if I misunderstood you.
I would change this PR, if the iD team would tell me that they will use it. But it looks to me as if only you - @artembert - need React support for a private fork. Again: Please correct me if I misunderstood this.
So I don't see why I should invest any more work into this. Like @bhousel said: "If you have a fork of iD where you're building UI elements in ReactJS, consider forking off the RapiD code instead"
If I understand @bhousel correctly, he won't merge this PR since he would prefer the solution he build in RapiD. @bhousel please correct me if I misunderstood you.
I'm not trying to block the merge, it's just that I don't really work on iD anymore. I don't merge things. OSM Foundation had me removed from the iD maintainer role last year.
Ah, OK. I thought, you were still a maintainer. Thank you for the clarification.
The main thing about this issue is adding JSX (or TSX) transpilation to the build - The changes in feature_info are just a demo. I used TypeScript for this purpose, @bhousel used babel. As said in my first comment, I prefer TypeScript over babel. But there are certainly good arguments on both sides, so in the end it doesn't really matter which way to go. Using TypeScript would obviously open the door to use TypeScript in the code base, which I would encourage as well.
A third possibility would be to replace the rollup build with esbuild, which as a positive sideeffect would dramatically speed up build time and would add JSX (or TSX) transpilation as well. If you like to go this way, I could contribute that as well.
So the questions to the maintainers would be:
- Do you even want React support at all?
- If yes: How should JSX/TSX transpilation work: rollup with TypeScript, rollup with babel or esbuild?
A third possibility would be to replace the rollup build with esbuild, which as a positive sideeffect would dramatically speed up build time and would add JSX (or TSX) transpilation as well. If you like to go this way, I could contribute that as well.
Ah yeah that React commit I linked to is several months old - we've already switched RapiD over to using esbuild, and it's pretty great. 👍
Our team is also working on modularizing the code and using TypeScript. This is a continuation of work I presented at SOTM-US in 2019. Send me an email and can chat privately if you want to know more.
Hi @til-schneider apologies for not noticing this earlier. This PR is a valid change, and I personally think React is awesome and it would be ideal that iD ui slowly got migrated towards this direction. Unfortunately it's best that for now we put this PR on pause, as I am concerned that taking in additional dependencies won't go well with the focus of current releases which is basically achieving stability from changes introduced through v2.20.0.
I'm paying close attention to the esbuild possibility and your help would be appreciated if it gets decided to perform the transpiler switch.
