Hi, and thanks for the PR. This one needs a bit more work (details inside the gemini review). I am curious: did your agent follow the agents.md file? If so, you might need to use a prompt like "use the ai knowledge base and explore how to create a new app." => it should query for guides, or browse through already existing demo apps to get the correct patterns.

PR Review: ticket-create-empty-viewport-app

Thank you for the submission. After a thorough review, I must reject this PR in its current state as it contains several fundamental architectural errors that violate the core principles of the Neo.mjs framework.

This is a failure on our part for providing a ticket that was not detailed enough for a contributor who may be new to the framework's multi-threaded architecture. We sincerely apologize for that.

To help clarify, let's walk through the issues.

1. The index.html Entry Point is Incorrect

The index.html file attempts to load app.mjs directly. This is the root of the problem, as app.mjs is the entry point for the app worker and cannot be loaded into the main browser thread. The main thread's entry point must always be src/MicroLoader.mjs, which bootstraps the entire worker environment.

2. The app.mjs Implementation is Architecturally Flawed

The code in app.mjs attempts to run logic that is meant for the main thread inside a worker, and in the wrong sequence.

• Execution Context: All application logic inside app.mjs must be placed within an exported onStart function, which the framework calls after the worker is initialized.
• window Access: The line window.__componentViewport = viewport; is a critical error. Web Workers run in a separate thread and do not have access to the window object.
• import Neo: The Neo namespace is a global created by the framework's bootstrap process; it should not be imported directly.

3. Unrelated File Included

This PR also includes the playwright-component.config.mjs file, which belongs to a different ticket. This indicates an issue with how the branch was managed.

---

Correct Implementation Examples

For clarity, here are two files from a working application that demonstrate the correct patterns:

1. apps/covid/index.html

   • Link: https://github.com/neomjs/neo/blob/dev/apps/covid/index.html
   • Shows: The correct way to set up the main thread entry point, loading MicroLoader.mjs and using neo-config.json.

2. apps/covid/app.mjs

   • Link: https://github.com/neomjs/neo/blob/dev/apps/covid/app.mjs
   • Shows: The correct structure for a worker entry point, exporting an onStart function that calls Neo.app().

---

Conclusion & Next Steps

Because of these fundamental issues, we cannot merge this PR as is.

However, if you would like to fix it, you can simply push new commits to your existing branch. This will automatically update the pull request—there is no need to create a new one.

We are truly sorry for the unclear instructions and we hope this detailed feedback is helpful. We appreciate you taking the time to contribute.

Hey @tobiu, thanks for the detailed feedback! I initially followed the simpler pattern just to get it running, and didn’t realize the architectural split between the main thread and the app worker. Went through the examples you mentioned and refactored everything the proper Neo.mjs way, index.html now loads MicroLoader.mjs, added the neo-config.json, and restructured app.mjs with the onStart() + separate viewport class. Also cleaned up the branch to drop unrelated files.

Should be aligned with the framework standards now. Appreciate the guidance!

✦ Hi @Aki-07,

Thank you for the very fast and accurate refactoring. You've correctly addressed the core architectural feedback—the index.html now uses the MicroLoader and app.mjs has the proper onStart structure. This is an excellent and necessary step forward for our new testing harness.

We are going to accept and merge this PR! Congratulations, and thank you for your contribution.

For full transparency and as a learning opportunity, we wanted to share a few minor refinements we'll be making in a follow-up commit. You don't need to do anything; this is just for your information. The framework has a central configuration file at src/DefaultConfig.mjs that provides sensible defaults, so we can simplify the neo-config.json to only include values that override those defaults.

1. In neo-config.json:

   • We'll remove the themes key. The default value from src/DefaultConfig.mjs is ['neo-theme-light', 'neo-theme-dark', 'neo-theme-neo-light'], which is exactly what we want for comprehensive component testing.
   • We'll also remove the mainThreadAddons: [] line. Setting this to an empty array overrides the default ['DragDrop', 'Navigator', 'Stylesheet'], which would prevent critical addons (like the one that injects CSS) from loading.
   • The useFallbackApi key is an unused property, so we'll remove that as well.

2. In the Viewport Architecture:

   • We'll simplify the structure by removing the EmptyViewport.mjs file and using the framework's base Viewport class directly. Since the subclass doesn't add new functionality, this is a bit more direct. The app.mjs will look like this:

     import Viewport from '../../../src/container/Viewport.mjs';
     export const onStart = () => Neo.app({
         mainView: {
             module: Viewport, // Use the module config directly
             id    : 'component-test-viewport'
         },
         name: 'ComponentTestApp'
     });
     

   • You correctly identified the need for a static id. While we often avoid static IDs in applications, it's the right choice for a test harness. It gives the Playwright runner in the main thread a stable, predictable handle to communicate with the viewport in the app worker.
   • We'll also remove the layout: 'fit' config. For testing individual components (like a button), a 'fit' layout can cause them to stretch to fill the screen in an unnatural way.

Again, these are just minor cleanups on our end. The fundamental architectural refactoring you did was excellent. This provides the solid foundation we need, and we're excited to build upon it for our new tests.

Great work, and thank you again for contributing to Neo.mjs