Fixing up latest development branch #39
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There were several breaking changes, new warnings, and changes in here. - Migrate from @import to @use in SCSS files. - replace “compileType” with “mode” in css-loader options - Set “namedExport” to false to preserve behavior of earlier css-loader versions - Switching from ‘sass’ to ‘sass-embedded’ to improve build performance.
This commit upgrades us to the latest ESLint 9. It adds better linting for TypeScript, in particular. It removes eslint-plugin-airbnb and replaces it with the ‘recommended’ configs for eslint, typescript-eslint, react, and react-hooks. It also adds the eslint stylistic config, and modifies it to match our existing code style. Subsequent commits will fix all the styling issues found in this commit; the repository wasn’t linting very consistently before this, so somehow a bunch of stuff slipped through.
I had a checkout with a ton of linting changes and just overlooked these files.
Because of the new config, a ton of eslint ‘ignore’ comments and such are no longer necessary.
Also getting rid of some unneeded eslint disable line comments
Also using ‘exec’ on a Regex instead of match.
The existing createConfig helper is not appropriate for the new version of ESLint for two reasons - one, it uses webpack-merge, while we need to use tseslint.config to merge config. Second, the argument of a single “configFragment” object is not what tseslint needs - it needs an array of ConfigWithExtends objects. For these reasons, I just split the lint config helper out into a separate function.
PluralRules and RelativeTimeFormat are widely available in browsers today, so these are no longer necessary.
Modules that need these functions should implement them themselves - frontend-app-account in particular. We don’t want to load these lists for the vast majority of the frontend that doesn’t need them. This also removes the third party dependencies that backed the lists.
It was duplicated in both the optional and required configs. Oops.
Our route objects have an optional “role” in the handle. This change to the types will allow that role to show up in auto-complete if a handle is added, along with a helpful comment explaining what it’s for.
The configure functions for auth, analytics, logging, and i18n are now named explicitly for the service they configure: - configureAuth - configureAnalytics - configureI18n - configureLogging The public API has already been changed, this just changes the internals to make it all easier to parse and follow through the code.
Makes it simpler to maintain our export lists.
- organizing ‘routing’ folder into utils file - fixing getUrlByRouteRole naming - adding isRoleRouteObject helper
A ‘role’ is a semantically meaningful identifer for a route or widget that fulfills a particular job in the site. Like the ‘login page’, ‘studio’, or ‘header’. We use roles to know what’s being displayed in the UI at any given time, particularly for UI Operation conditions. A UI operation can now use an “active” condition to perform an operation only if a particular role or set of roles is active.
Still need to make a proper test setup for the iframe widget.
It’s an app, not a config.
Nothing needs these at the moment, but they’ll likely be helpful and mirror other helpers that are already in the file.
For simplicity’s sake, we let the type of the layout prop be ReactNode and then weed out any sub-types that aren’t an actual layout component, i.e., boolean, null, and undefined.
…NTICATED_USER_CHANGED The useOperations hook can use getSlotOperations to set its value immediate without waiting for useEffect to fire. It also should NOT listen to AUTHENTICATED_USER_CHANGED; the user’s auth state changing does not affect the unfiltered list of operations for a slot, so listening for it would cause unnecessary re-renders.
Apps generally have custom config values that they need to supply on load; this allows them to supply that config as part of the App, and the Shell will make it available through the getAppConfig(id: string) function after processing it. This helps differentiate between the site’s core config available through getConfig and App-specific config now available through getAppConfig.
This is temporary, and just to show how it works.
The useActiveRouteRoleWatcher hook needs to exist inside a router, which isn’t always true for tests. This is functionally equivalent, but means test suites throughout the platform will not have to change. Also moved usTrackColorSchemeChoice too, which leaves AppProvider focused on its contexts.
Apparently you don’t import types used in the declaration? Wild.
This fixes the test-project build.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes a couple of fix-ups on top of #37.