Problem with node paths

vrozaev commented 7 years ago

Hello! I trying make a demo application for intern-visual and I have a problem with paths: Error: Failed to load module fs from /Users/rozaev/work/intern-visual-demo/fs.js (parent: intern-visual/assert).

How-to reproduce:

git clone git@github.com:rozaev/intern-visual-demo.git 
cd intern-visual-demo 
npm install && npm test

I did something wrong or intern-visual didn't work standalone right now? It seems that problem is somewhere in loader. Maybe intern-visual should use "intern/dojo/node!" for loading node modules?

devpaul commented 7 years ago

Howdy rozaev,

I'm seeing

Error: Failed to load module fs from /Users/pshannon/sss/intern-visual-demo/fs.js (parent: intern-visual/assert)

which means you'll definitely want to use intern/dojo/node!. Admittedly it's confusing because the package is compiled to use a UMD module format rather than a commonJS format. We should switch it to export in CJS format to make it more clear.

devpaul commented 7 years ago

Closed for #2

vrozaev commented 7 years ago

Howdy @devpaul ! It seems that you are right about that:

which means you'll definitely want to use intern/dojo/node!. Admittedly it's confusing because the package is compiled to use a UMD module format rather than a commonJS format. We should switch it to export in CJS format to make it more clear.

But I want notice that now intern-visual just broken, even if I will use intern/dojo/node!intern-visual. Seems that I can't do nothing until we switch export format. If you use

intern/dojo/node!intern-visual

It didn't work and I you will got

TypeError: define is not a function.

Which happen because we didn't have define in CommonJS.

My message is: hey guys it's never worked before 😄 , we should fix it asap until someone else notice that.

devpaul commented 7 years ago

Sorry, I took a quick look at this one and missed a large part of the problem. intern-visual requires a loader that can handle loading both node and AMD modules. We're using the @dojo/loader in our tests to handle this:

https://github.com/theintern/intern-visual/blob/master/tests/intern.ts#L15-L16

vrozaev commented 7 years ago

@devpaul okay, I got the idea. But maybe can do something with it? I mean, look at form my point of view: for example I am a new developer, I want to use intern-visual. I add intern-visual to my existing intern tests and surprise: I should also define custom loader.

Can we do something and make set-up process more user-friendly?

Other intern features didn't require custom loaders. (Please correct me if I am not right). Can we make things more simple? I love intern because it's allowed me do many things out the box, it is his killer-feature.

Anyway, thank you for your pull request, seems it's works. I will continue to write demo app.

devpaul commented 7 years ago

It may be possible to use Intern's default loader. We would need to use an AMD map for any node module so it mapped to something like

map: { 
    'intern-visual': { 
        fs: 'intern/dojo/node!fs'
        mkdirp: 'intern/dojo/node!mkdirp'
    } 
}

Dojo's AMD loader is flexible enough to do whatever you need, even if you have to write an AMD plugin to do it, it's just not always practical. In this case it was easier to use the @dojo/loader.

As far as providing a better user experience, I would love to see that. We would need something that reduces configuration time, maybe reduces external dependencies, and any PR would need to avoid using AMD plugins like intern/dojo/node! in order to be compatible with Intern 4.

We should see if it's possible to load intern-visual using Intern's loader. Then maybe we could add a feature to intern-cli that allows you to add these plugins (intern-visual and intern-a11y) to an intern.ts configuration file and automatically provide configuration when the default Intern loader or the @dojo/loader is used?

theintern / visual-plugin

Problem with node paths #1