refactor: Move Sprocket JS files to Webpack (Part 2)

[arkirchner]

Code editor modules are moved with minimal changes.

The editor is tested in the exercise implimentaion page. Admin exercise page and the RfC page.

Relates to #3021

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.08%. Comparing base (721f80e) to head (c60094e).

Additional details and impacted files

@@                            Coverage Diff                             @@
##           ak/move_sprocket_assets_to_webpack_part_1    #3030   +/-   ##
==========================================================================
  Coverage                                      70.08%   70.08%           
==========================================================================
  Files                                            215      215           
  Lines                                           6850     6850           
==========================================================================
  Hits                                            4801     4801           
  Misses                                          2049     2049           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[arkirchner]

Routes.js needs the page to be rendered before execution. Calling this on execution ensures the page is present.

[ibrahimkonuk]

Looks good to me, but haven't tested locally

[christophblessing]

What about the failing specs? Your changes seem not functional.

[MrSerth]

On my machine, core aspects of the application are broken, such as the navigation. In addition, errors are given on the console, which reminds me of #2953 (review).

[arkirchner]

I am not quite sure why Routes needs to be included here explicitly. I am a bit worried why this worked locally for me (now also failing for me).

We are setting Routes on the window in the application.js before requiring the editor.
https://github.com/openHPI/codeocean/blob/main/app/javascript/application.js#L77

This breaks my understanding of how JavaScript should work for me. Is Webpacker moving JS around?

[christophblessing]

I'm not very familiar with the code base. But from your description of the problem I would say that it's better to explicitly import the Routes in the all modules they need it.

Attaching something to the window leads to several problems like global namespace pollution, potential module system conflicts and challenges in maintainability (difficult to track for developers). If CodeOcean has a module system in place, we should also use it. Attaching objects to the global space should be avoided.

[ibrahimkonuk]

It could be a cached local build or browser script cache which is why it worked initially. I just read in this article here that it might be a scoping issue and explicitly importing looks like the best way.

[arkirchner]

@christophblessing
I agree that it would be better to implicitly import everything. In this transition, this is a bit difficult because this would mean a large code change. I want to first move the JavaScript sprockets and then refactor them with more and more implicit imports.

@ibrahimkonuk
Thank you for linking this article. I will look deeper into how webpack orders the objects.

I want to avoid some on of solution. Implicitly importing Routes here and using the window version in other places might work but will cause difficult issues down the line. I will keep this as a draft while researching.