search

openedx / open-edx-proposals

Proposals for Open edX architecture, best practices and processes
http://open-edx-proposals.readthedocs.io/
Other
44 stars 32 forks source link

OEP-11 Frontend Technology Updates (Epic) #480

Open robrap opened 1 year ago

robrap commented 1 year ago

Problem: OEP-11: Front End Technology Standards is outdated and doesn't reflect current technology usages as well as directional changes we're making or need to make moving forward (for example, moving to Typescript as a dependency of upgrading to React 19). Thus, the OEP needs to be updated to reflect changes. Please add comments in this ticket if you will make a PR to update one of the following technology types, or if you have found additional sections that need updating.

Changes Requested

Original comment

There is a long-standing, but undocumented stance that we don’t want new code in legacy server-side templates, and that all new code should be in an MFE. In theory, it would be great to require an ADR or comments explaining any exception to this rule.

It was acknowledged that we don't have this documented, and that OEP-11: Front End Technology Standards would be a good place for it. The sections “1. Use React and Redux” and the intro to “12. Server-side content should be rendered with Django Templates” may be affected. However, ideally, this decision would be made much more explicit.

FYI: @sarina

sarina commented 1 year ago

@arbrandes or @brian-smith-tcril have you seen this proposed change? Do you have any thoughts?

arbrandes commented 1 year ago

Hadn't seen it yet. And now that I have, I have no objections. On the contrary: this stance should definitely be documented!

robrap commented 1 year ago

@arbrandes: FYI: Another OEP-11 issue may be coming regarding TypeScript, because it sounds like sentiment has been changing on that? I'm not sure if someone will create it, but this at least notes it for you.

arbrandes commented 1 year ago

@robrap, thanks! And for reference, Paragon has already started to go down the typescript road.

robrap commented 1 year ago

@arbrandes: Not sure if you want another issue to collect other OEP-11 updates, or if you want to use this issue? Others are providing thoughts in a private Slack, and I tried to get that moved to a ticket, but I don't see that happening. Here is an example:

I definitely think we should revisit that OEP. There are a couple of points here I think would be interesting.

  • TypeScript should at least not prohibited in there, for sure
  • We should move from "you should use redux" to "you should use redux, but only if you really need it" and link to articles on when to use redux.
  • In terms of testing, I think we should mention react-testing-library as the go-to tool with Enzyme more "as necessary" (best argument for that is that enzyme does not support hooks in any reasonable way)
arbrandes commented 1 year ago

@robrap, I think this issue is a perfect place to discuss this topic. Even if some conversations happen elsewhere, we should at least try to bring the relevant bits here: just as you just did (thanks!).

jesperhodge commented 1 year ago

Thanks @robrap . Enzyme was already mentioned above. However, we are facing the problem now that Enzyme will likely never support React 18. So we want to deprecate enzyme, and that is something that should be reflected in the OEP.

To quote @adamstankiewicz , who brought this up: "enzyme is basically dead and has no React 18 support: https://dev.to/wojtekmaj/enzyme-is-dead-now-what-ekl Looks like any repo that wants to use React 18 won't be able to use enzyme, so any existing enzyme tests in the repo will have to be migrated to react-testing-library (e.g., we're going to start migrating Paragon's enzyme tests to RTL to be able to support React 18 in Paragon)."

It would be fairly relevant to clearly state in the OEP that we are rejecting and deprecating enzyme starting now. I don't see another option at the moment.

sarina commented 1 year ago

I think these different ideas would go faster if made in separate PRs to the OEP. For example, it seems obvious that a PR can be made about enzyme today, and it would be non-controversial. I think better to have smaller, separate PRs rather than an omnibus.

arbrandes commented 1 year ago

Probably best to wait for https://github.com/openedx/open-edx-proposals/pull/518 to merge, at this point, and take it from there.

feanil commented 11 months ago

FYI https://github.com/openedx/open-edx-proposals/pull/518 has merged.

sarina commented 4 months ago

Updated the description to collect all comments like in an Epic ticket. Posted to FWG channel in Slack. Hopefully collaboratively we can get this done.

Anyone picking up an issue, feel free to tag me for review.