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

Moving some code around #1055

Merged
merged 20 commits into from
Mar 31, 2023
Merged

Moving some code around #1055

merged 20 commits into from
Mar 31, 2023

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Mar 27, 2023

Description

This is a low-priority PR (conflicts should be reduced before merging this). I'm continuing the ongoing repo and code cleanup by moving some files and code around - in order to hopefully make a more meaningful structure.

In summary:

  • The LayoutNode, LayoutPage, LayoutPages and LayoutObect classes has been moved into their own respective source files (out from hierarchy.ts).
  • Moving all hooks into their own files, and moving all hook files to src/hooks instead of a combination of src/common/hooks and src/components/hooks.
  • Moving everything out of src/shared. Most files, from src/shared/resources has been moved into src/features, where their most closely resembling counterparts reside in the newer repo structure. These 'resources' mostly contain sagas to load app content into the redux store.
  • Combining the minor src/sagas, src/reducers and src/store into src/redux. (To reduce the number of small top-level folders, and all of these belong to store/state initialization).

Later additions:

  • Moving even more files around (reducing depth of saga files in src/features), thanks to suggestion by @bjosttveit.
  • The change above re-introduced problems I saw earlier with saga slices running when being imported, thus causing them to rely on import order to work correctly (some imported values that are not declared yet will be undefined). For that reason, I rewrote all the slices to functions generating slices, and bootstrapped everything in index.tsx so that practically no code starts running until the JS file has been evaluated and all imports are resolved.
  • With import order issues resolved, more of the files could be moved into more logical places.

Related Issue(s)

Verification/QA

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Changes/additions to component properties
    • Changes are reflected in both src/layout/layout.d.ts and layout.schema.v1.json, and these are all backwards-compatible
    • No changes made
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

@olemartinorg olemartinorg added kind/other Pull requests containing chores/repo structure/other changes ignore-for-release Pull requests to be ignored in release notes labels Mar 27, 2023
@olemartinorg olemartinorg self-assigned this Mar 27, 2023
Ole Martin Handeland added 6 commits March 27, 2023 19:45
…ers/* to src/features/wrappers (dropping dataLists, as that's causing trouble)
…it comes before 'src/features/form' in the import order. This happens because of some dependency that causes formDataSlice.ts and formLayoutSlice.ts not to load it correctly. Renaming the whole thing to just 'lists' mitigates the problem.
…ndex.ts` into `src/redux` (along with `src/utils/sagaSlice.ts`)
@olemartinorg olemartinorg marked this pull request as ready for review March 27, 2023 20:33
Copy link
Member

@bjosttveit bjosttveit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of great changes! Have you considered getting rid of some of the folders containing a single saga-file?

A few examples, but there are many:

image

@olemartinorg
Copy link
Contributor Author

@bjosttveit Great point! I didn't go that deeply into it, but I agree with you 100%. I'll check.

Ole Martin Handeland added 6 commits March 30, 2023 10:18
# Conflicts:
#	src/layout/FileUploadWithTag/EditWindowComponent.tsx
…mports have been resolved, thus getting rid of the need to import code in the 'correct' order for our app to work. This now starts running the app on DOMContentLoaded.
…e of import order requirements (now that they are gone)
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 35 Code Smells

63.8% 63.8% Coverage
0.0% 0.0% Duplication

@olemartinorg olemartinorg merged commit db6e6e2 into main Mar 31, 2023
@olemartinorg olemartinorg deleted the chore/moving-files branch March 31, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release Pull requests to be ignored in release notes kind/other Pull requests containing chores/repo structure/other changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants