Switch to local PrimeVue imports

[maximilianoertel]

Proposed changes

This PR replaces the existing global PrimeVue component imports in favor of local imports and removes unused components. The use of local component imports should improve bundle size and performance, reducing the overall load on the application. Additionally, this approach enhances maintainability by making dependencies more explicit within each component.

Whilst still too large, the application entry point is now reduced by approximately 1 MB uncompressed and 226 kB gzipped.

- dist/assets/index-BOcMm_Dr.js                       3,101.02 kB │ gzip: 712.68 kB │ map: 9,624.27 kB
+ dist/assets/index-CyOyLE4r.js                       2,076.42 kB │ gzip: 486.59 kB │ map: 6,723.98 kB

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactoring (non-breaking change that does not add functionality but makes code cleaner or more efficient)
• Documentation Update
• Tests (new or updated tests)
• Style (changes to code styling)
• CI (continuous integration changes)
• Repository Maintenance
• Other (please describe below)

Checklist

• I have read the guidelines for contributing.
• The changes in this PR are as small as they can be. They represent one and only one fix or enhancement.
• Linting checks pass with my changes.
• Any existing unit tests pass with my changes.
• Any existing end-to-end tests pass with my changes.
• I have added tests that prove my fix is effective or that my feature works.
• If this PR fixes an existing issue, I have added a unit or end-to-end test that will detect if this issue reoccurs.
• I have added JSDoc comments as appropriate.
• I have added the necessary documentation to the roar-docs repository.
• I have shared this PR on the roar-pr-reviews channel (if I have access)
• I have linked relevant issues (if any)

Justification of missing checklist items

n/a

Further comments

n/a

Resolves yeatmanlab/roar-project-management#422

[cypress]

roar-dashboard-e2e    Run #8752

Run Properties:   Passed #8752  •  20e7951b15: E2E Tests for PR 918 "Switch to local PrimeVue imports" from commit "20e7951b158...

Project	roar-dashboard-e2e
Branch Review	ref/primevue-local-imports
Run status	Passed #8752
Run duration	06m 09s
Commit	20e7951b15: E2E Tests for PR 918 "Switch to local PrimeVue imports" from commit "20e7951b158...
Committer	Maximilian Oertel
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	14
View all changes introduced in this branch ↗︎

[github-actions]

Visit the preview URL for this PR (updated for commit 20e7951):

https://roar-staging--pr918-ref-primevue-local-i-0vkcqbyx.web.app

_{(expires Tue, 05 Nov 2024 18:20:30 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 2631e9c58fd0104ecbfddd72a62245ddac467460}

[ksmontville]

Looks good, but I am wondering what (if any) effect this will have on our Cypress component tests? For instance, here is our custom cy.mount() command from component.js

Cypress.Commands.add('mount', (component, options = {}) => {
  const app = createAppInstance();

  options.global = options.global || {};
  options.global.plugins = options.global.plugins || [];
  options.global.components = options.global.components || {};

  // Add the Vue app plugins to the Cypress context
  plugins.forEach((plugin) => {
    if (Array.isArray(plugin)) {
      options.global.plugins.push(...plugin);
    } else {
      options.global.plugins.push(plugin);
    }
  });

  // There is some context duplication between loop above and this loop
  // But without this redundancy, some app context is not available in the Cypress context (namely i18n)
  // Unsure why, need to investigate further

  // Add the Vue app components, directives, and plugins to the Cypress context
  options.global.plugins.push({
    install(appInstance) {
      appInstance._context.components = app._context.components;
      appInstance._context.directives = app._context.directives;
      appInstance._context.provides = app._context.provides;
    },
  });

  return mount(component, options);
});

This custom command was able to mount any of our components and their dependencies.

Since we were registering (most) components globally before these PR changes, every component that mounted within Cypress had access to (most) other components, even ones that it did not need in order to perform its function.

With these PR changes, each MyComponent.vue file imports the other components that it needs. So now each component will mount within Cypress with only the components that it needs in order to function, is that correct?

I just want to avoid a situation where we would need bespoke cy.mount() commands for specific components that we would like to test.

[maximilianoertel]

This PR shouldn’t impact the component test configuration you set up, @ksmontville. It simply moves the previously globally registered PrimeVue components to be imported directly within the individual components where they’re used. With PrimeVue components now loaded locally inside each relevant component, we can remove those global component registrations from the setup.js file you added during the initial component test setup.

More generally, the code snippet you shared theoretically shouldn't be necessary, as component tests are ideally designed to test components in isolation. This works really only when the components themselves are structured in a testable way, which, as you know, isn’t quite the case here — hence, the custom mount command that you had to introduce.

In other words, ideally we'd move away from needing the custom cy.mount command by refactoring our components and the local imports are already a step towards that. I can take a closer look at the component tests after this PR in my time between tasks if that's alright with @richford?

I hope that answers your question and if not, please let me know!