Code Monkey home page Code Monkey logo

Comments (16)

twolfson avatar twolfson commented on July 2, 2024 1

Ah, I see. Yea, there's a discrepancy between how we would expect to load the JS files. You are loading via:

<script src="./app/app.js"></script>

whereas I would expect to use

<script>require('./app/app.js');</script>

or have a browser.js which then requires an application constructor and initializes it (so the tests can have the same behavior and we avoid globals altogether).

It's been a while since I've touched this repo but I recalled setting up a similar gist to verify we had the majority of the behavior matched. However, now that I look at it again we never tested these different loading mechanisms:

https://gist.github.com/twolfson/c6213aa59f7c3f6477

I want to take some time to test further, reflect on which behavior should be considered the default/why, and likely add an option for the opposite setup (e.g. requireRelativeToPage). Here are my current thoughts:

  • I think I have a bias towards relative to the script files (i.e. the norm for Node.js as we strongly avoid globals and prefer consistent requires behavior across sibling files)
  • karma was built by the Angular team so maybe demos/tutorials would lead to a setup more similar to the one you provided

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

Could you elaborate more on "require in renderer process use index.html path as base whereas your plugin use the .js file as base."? Maybe provide some example code so I can understand better

from karma-electron.

danforbes avatar danforbes commented on July 2, 2024

I think I might understand this question. I have an Angular controller defined in <application root>/app/home/home.ctrl.js. In this controller, I say:

var foo = require(./app/foo);

And this works great when I run my application by simply spawning electron in the directory containing my main.js files. However, when I run my Karma tests, I get an error that tells me;

Cannot find module '<application root>/app/home/./app/log'

Is this because your preprocessor is setting the __dirname variable?

I have a temporary workaround. Rather than using require to import my module, I source it in my index.html file. In the file I'm sourcing, I just set process.<some property> to some value (e.g. a function, an object) that will allow me to access the method(s) defined in my module.

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

The current require is polyfilled to match the experience someone should expect from electron/Node.js. The require resolves relative to the current script file (subsequent requires to a script loaded from index.html are relative to the file itself, not the initial require context). To reiterate:

views/index.html:

<!-- Loads `views/../js/foo.js` (i.e. `js/foo.js`) -->
<script>require('../js/foo');</script>

js/foo.js:

// Loads `./bar.js` (i.e. `js/bar.js`)
require('./bar');
// This is the context we load scripts as
//    That is: The file you are loading in `karma` is a JS file
//    someone wouldn't/shouldn't expect it to load from the HTML context
//    as the file isn't HTML

The reason we need to polyfill __dirname and __filename is karma executes tests from a localhost server. Since this is an HTTP context and not file://, electron doesn't know how to orient itself so we must redefine those would-be values

from karma-electron.

danforbes avatar danforbes commented on July 2, 2024

Well, that's all fair enough, I suppose, but I think the reason that this issue was raised is because there seems to be an incongruity between the behavior in the normal runtime environment versus the testing environment. That is, paths that resolve in the normal runtime environment do not resolve in the testing environment and vice versa.

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

Ah, I think I understand now. I will rephrase the problem to verify we are on the same page:

  • karma hosts a local file server so assets can be loaded as though they are from a web application
  • electron uses a require which loads either absolute file paths, relative file paths, or third party libraries from a node_modules directory

The question seems to be something like: Can we add basePath to the require resolution path?

My answer would be: We could but I would prefer to avoid doing so as it would lead to unpredictable development/testing behavior. As a developer, I would expect a require for ./foo/bar.js to resolve relative to the same file in both a normal electronand testing environment. If we were to add basePath to the require resolution path, then we could either load separate files or get a miss in development but hit in testing.

from karma-electron.

danforbes avatar danforbes commented on July 2, 2024

Thanks for the response - glad I was able to help clarify!

I don't have the same depth of knowledge that you have, but here're my two cents: the testing environment should behave exactly like the normal runtime environment. I totally hear what you're saying, vis-a-vis reasonable developer expectations, but the aim of a testing environment should be to mimic the normal runtime environment, regardless of whether or not the normal runtime environment conforms to "reasonable" expectations.

You seem to indicate that "fixing" this problem could result in incongruous behavior between the normal runtime environment and the testing environment, but as far as I'm concerned the problem we're discussing right now is that there is an incongruity of behavior between the normal runtime and testing environments.

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

Nope, the behaviors should currently be the same as karma-electron was designed under that philosophy (i.e. karma-electron matches electron as closely as possible). My current interpretation of the issue is to add another path to require which is karma's basePath option

That being said, I could be wrong about missing some inconsistency between electron and karma-electron's polyfill. If you have any examples, please let us know =)

from karma-electron.

danforbes avatar danforbes commented on July 2, 2024

I've prepared some code to demonstrate the problem I'm trying to describe.
DanForbes/electron at test/karma-electron/require

/app/home/home.ctrl.js requires /app/node-scripts/my-script.js, which resolves properly in the Electron runtime environment (the "Whoa, this works great message!" comes from the required file). When I try to run my tests, though, I get the following error message:

Uncaught Error: Cannot find module '<project root>/app/home/./app/node-scripts/my-script' at module.js:457

Let me know if I'm missing something, doing something wrong or if you have any questions.

from karma-electron.

danforbes avatar danforbes commented on July 2, 2024

Glad I was able to clearly illustrate the issue. Also, thanks for describing the way in which you would have structured things in order to avoid this issue. Although I'm a software engineer by profession, I'm not employed as a web dev and am keenly aware of the fact that I don't write web development code like a professional...so I'm happy to hear how a more seasoned web developer would structure things!

As the repository linked in my last comment indicates, I followed a tutorial to build the "bones" of my minimal Electron app. As you can see in the fourth code block in the Controller & Template section, I am sourcing my scripts using the same syntax they are. Do you not agree with the way they are doing things? Is there something fundamentally different about my use case that requires this kind of script sourcing to be done using different mechanisms?

I understand how using this kind of syntax:

<script>require('./app/app.js');</script>

Would "work", but also create globals and probably be undesirable. However, I'm not as clear on your second suggestion, regarding the use of a browser.js script. Can you give me a little bit more information about how I might use that approach in the specific use case illustrated in the example with which I provided you? Is there an existing example somewhere out there that illustrates the methodology you're trying to describe?

Thanks in advance...please let me know if I can provide any additional information or if there's anything at all I can do to help!

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

Sure, here's some clarification/guidance

The require syntax won't create any globals but <script src=" will:

  • require executes in an isolated context (i.e. any top level variables like var foo = 'bar'; won't leak to the global scope)
  • <script src=" uses the default browser behavior so any top level var foo = 'bar'; will become window.foo = 'bar';

Next we have dependency management in the application. The tutorial you link to is following conventions that are ideal for a browser environment and doesn't use require in any of its browser code.

The gist of what's going on is:

  • Dependencies are loaded independent of order (i.e. anywhere after angular.js in index.html works)
  • The scripts are using Angular's dependency management (i.e. angular.module('app').controller)
    • This is popular because it supports dependency injection during testing and fits into the Angular ecosystem
    • These don't run any code, only declare it

If you want to continue using that paradigm, it's fine but it should also use the same dependency management system and probably not mix the 2 as it will issues (e.g. confused developers, 2 separate dependency trees)

In constrast, require handles both loading modules and dependency management. Here's an example which dodges <script src=", it uses import instead of require but the gist is the same. I should also mention that the tutorial you used is Angular@1 whereas Angular@2 has been released.

http://angular-tips.com/blog/2015/06/using-angular-1-dot-x-with-es6-and-webpack/

In other frameworks like React, they use require directly (with Browserify or Webpack for the browser) and don't have a separate dependency manager. Here's a simple example (see index.html and src):

https://github.com/toranb/react-gulp-browserify-example

With electron, we can directly require src/app.js from index.html instead of bundling first

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

Updates to the actual issue: I have added a reference application to this repo based on the previous gist:

https://gist.github.com/twolfson/c6213aa59f7c3f6477

https://github.com/twolfson/karma-electron/tree/4.1.2/test/reference

The resulting values match our discussion so far (as we did the same work but outside of this repo):

https://github.com/twolfson/karma-electron/blob/4.1.2/test/reference/js/src-main.js

https://github.com/twolfson/karma-electron/blob/4.1.2/test/reference/js/require-main.js

The current unmodified behavior for electron with karma-electron are as follows. These are caused by karma using http:// instead of file://:

  • __filename is /home/todd/github/karma-electron/node_modules/electron/dist/resources/electron.asar/renderer/init.js
  • Only core node modules can be loaded (e.g. fs, electron)
  • Relative node modules will resolve from __dirname (i.e. /home/todd/github/karma-electron/node_modules/electron/dist/resources/electron.asar/renderer)

I think the default expected behavior would be (e.g. when <script src= is used):

  • __filename is /home/todd/github/karma-electron/node_modules/karma/static/context.html
  • Local node modules work (e.g. anything in /home/todd/github/karma-electron/node_modules)
  • Relative node modules will resolve from __dirname (i.e. /home/todd/github/karma-electron/node_modules/karma/static/context.html)

Then we have the Node.js-like behavior (e.g. when <script>require is used):

  • __filename is /path/to/my-script.js
  • Local node modules work (e.g. anything in node_modules directory near /path/to/my-script.js)
  • Relative node modules will resolve from __dirname (i.e. /path/to)

In addition to this, there's the requested behavior which is to allow a custom context.html location -- we could allow this to be done via a customContextFile but I feel like that's cruel as it requires creation of extra files.

To solve these problems, I plan on doing the following:

  • Updating preprocessor to use default expected behavior
  • Add a configuration option which loads our scripts via require (e.g. electron.loadScriptsViaRequire)
    • This will be 1:1 with our expectations as module.parent will be properly defined
  • Add a configuration option which overrides filepath for context.html (e.g. electron.__filenameOverride)
    • This will override both /context.html and /debug.html for convenience

I'm not certain of the electron namespacing or not but I will see what other repos do before deciding

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

The first patch has been released in 5.0.0 (i.e. client.loadScriptsViaRequire). The __filenameOverride should be much quicker now that the majority is done

from karma-electron.

twolfson avatar twolfson commented on July 2, 2024

We have added support for client.__filenameOverride in 5.1.0. @thibaultfriedrich and @danforbes The configuration you will probably want is:

module.exports = function (config) {
  config.set({
    client: {
      // Override top level `__filename` to be `/home/.../my-electron-app/index.html`
      //   where `__dirname` is `/home/.../my-electron-app`
      __filenameOverride: __dirname + '/path/to/index.html',
    }
  });
};

Thanks again for the bug report and clarification

from karma-electron.

friedrith avatar friedrith commented on July 2, 2024

Hi, sorry for the silence, but I have just noticed that my github was using a trash email and so I have never been notified. I know it is quite late but I can provide more examples if needed. Thanks you @danforbes and @twolfson for having kept this thread alive.

from karma-electron.

danforbes avatar danforbes commented on July 2, 2024

Thanks for all the information and quick fix, @twolfson!

from karma-electron.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.