#11561 Load all components from arches apps before looking in core

[johnatawnclementawn]

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)

Description of Change

Issues Solved

Closes #11561

Checklist

• I targeted one of these branches:
  • dev/8.0.x (under development): features, bugfixes not covered below
  • dev/7.6.x (main support): regressions, crashing bugs, security issues, major bugs in new features
  • dev/6.2.x (extended support): major security issues, data loss issues
• I added a changelog in arches/releases
• I submitted a PR to arches-docs (if appropriate)
• Unit tests pass locally with my changes
• I added tests that prove my fix is effective or that my feature works
• My test fails on the target branch

Ticket Background

• Sponsored by:
• Found by: @johnatawnclementawn
• Tested by: @
• Designed by: @

[jacobtylerwalls]

+1 tested, will give Christopher a chance to take a look, too.

[jacobtylerwalls]

So this definitely improves the situation, and I'm happy if we merge it, but I just noticed that this is still going to return wrong results in terms of overrides by application ordering if something that shouldn't override first gets to override first simply because it's installed with -e and meets one of the try/catch conditions instead of progressing to the next loop iteration over the site-packages paths. We probably want to refactor this so that each of the 9 try/catch blocks tries a pair of require() calls, once in site-packages, once in the editable path.

[chrabyrd]

👀

[chrabyrd]

@johnatawnclementawn @jacobtylerwalls

yeah I can't wait to get rid of this file. Pretty sure it's just there to support KOJS stuff. Anywho I gave this another going-over to avoid logic against comparing the length of ARCHES_APPLICATIONS. This lgtm but should be looked at again by you two 👍

@jacobtylerwalls could you give a reproducable example of you're talking about? Trying to grok it.

[jacobtylerwalls]

@jacobtylerwalls could you give a reproducable example of you're talking about? Trying to grok it.

Sure, the idea is that:

• your ordered arches applications are FOO, BAR, BAZ
• BAR and BAZ contain the component
• FOO and BAR are installed in site-pacakges, but BAZ is installed with -e, so it gets a linked_application_path

Expect: bar component is found
Actual: baz component is found, because we iterate like this:

check FOO in site packages
     check all editable paths for all applications

so we find baz before continuing the loop to fin bar in site-packages. Granted I haven't set this up with to actually test, just seemed this way from a code read.

[chrabyrd]

@jacobtylerwalls could you give a reproducable example of you're talking about? Trying to grok it.

Sure, the idea is that:

• your ordered arches applications are FOO, BAR, BAZ
• BAR and BAZ contain the component
• FOO and BAR are installed in site-pacakges, but BAZ is installed with -e, so it gets a linked_application_path

Expect: bar component is found Actual: baz component is found, because we iterate like this:

check FOO in site packages
     check all editable paths for all applications

so we find baz before continuing the loop to fin bar in site-packages. Granted I haven't set this up with to actually test, just seemed this way from a code read.

ugh. yeah. I think this will require a change to webpack as there's no guarantee LINKED_APPLICATION_PATH_${num} matches the ARCHES_APPLICATION iteration value, so a double require block won't work.

However, like you I'm happy to have this merged in and take care of the hardening in a followup, as anything that requires user action doesn't belong in a patch release if we can help it 😅