Skip to content
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

Update OEP-11 to clarify past decision around MFE development #480

Open
robrap opened this issue May 15, 2023 · 10 comments
Open

Update OEP-11 to clarify past decision around MFE development #480

robrap opened this issue May 15, 2023 · 10 comments
Assignees
Labels
documentation Relates to documentation improvements

Comments

@robrap
Copy link
Contributor

robrap commented May 15, 2023

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
Copy link
Contributor

sarina commented May 16, 2023

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

@arbrandes
Copy link

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

@robrap
Copy link
Contributor Author

robrap commented May 18, 2023

@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
Copy link

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

@robrap
Copy link
Contributor Author

robrap commented May 19, 2023

@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
Copy link

arbrandes commented May 23, 2023

@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!).

@arbrandes arbrandes self-assigned this May 23, 2023
@jesperhodge
Copy link
Member

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
Copy link
Contributor

sarina commented May 23, 2023

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
Copy link

Probably best to wait for #518 to merge, at this point, and take it from there.

@feanil
Copy link
Contributor

feanil commented Oct 17, 2023

FYI #518 has merged.

@arbrandes arbrandes added the documentation Relates to documentation improvements label Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Relates to documentation improvements
Projects
Status: Backlog
Status: Backlog
Development

No branches or pull requests

5 participants