Closed Bug 1712507 Opened 4 years ago Closed 4 years ago

Investigate using tsarch to validate architecture constraints in frontend

Categories

(Webtools Graveyard :: Pontoon, task)

Tracking

(Not tracked)

RESOLVED MOVED

People

(Reporter: axel, Unassigned)

References

Details

Attachments

(1 file)

There's a tool called tsarch which allows to test things like dependencies etc that should come with an architecture.

We should investigate if we have such constraints, and if so, if they're reasonable to validate automatically.

I've made up two potential rules, but I'm not sure if they're actually any good:

  • Things in src/core/** should not depend on things in src/modules/**
  • There shouldn't be circular dependencies between high-level features

Both of these can be implemented in tsarch, and, both of them fail. I'll draft a PR to have something to talk about.

To add some context, I looked at the feature cycles, trying to figure out what my speculations mean in practice, and if the cases that trigger them are bugs or features or nits.

First, core/api -> core/locale -> core/api.

This is triggered by core/api/machinery taking Locale as input. And the GraphQL-based types are defined in core/*/actions, for both Locale and Project. In the case of Locale, the data is even processed for cldrPlurals. So we have a GraphQL version of the Locale type going from core/api/locale to core/locale, and then core/api/machinery taking the processed type from core/locale.

Bug or feature or nitpicking?

My take would be to move the definition of Locale and Localization from core/locale/actions into core/api/types, and the processing of the GraphQL response into core/api/locale and /project. Maybe have a generic GraphQL handler even in core/api/base.

Just briefly glanced at core/editor -> modules/history -> core/editor. One trigger here seems to be

const historySelector = (state): HistoryState => state[history.NAME];

in core/editor/selectors.

Bug or feature or nitpicking?

The reverse dependency is that modules/history/actions uses core/editor/actions to change the status of prior translations. I kinda think that's OK.

My take would be to have a core/history feature with both state and selector. No idea about actions, the UPDATE one is the one that circles, so I'd say that action for sure not?

Big picture, is this worth it? For one, both, either?

°tsarch° currently isn't maintained, https://twitter.com/ArghRich/status/1420719372641914883 says

There‘s currently no maintainer. Maybe that‘ll change in a few months because my next project is Ts again and I plan to use it widely.

Might still be interesting to look at the rules and violations of them to see what our frontend arch is, or should be. But maybe the PR isn't useful to actually merge.

*This bug has been moved to GitHub.* *Please check it out on https://github.com/mozilla/pontoon/issues.*
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → MOVED
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: