Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with node paths #1

Open
vrozaev opened this issue Feb 14, 2017 · 6 comments
Open

Problem with node paths #1

vrozaev opened this issue Feb 14, 2017 · 6 comments

Comments

@vrozaev
Copy link
Contributor

vrozaev commented Feb 14, 2017

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 [email protected]: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
Copy link
Contributor

devpaul commented Feb 14, 2017

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
Copy link
Contributor

devpaul commented Feb 14, 2017

Closed for #2

@devpaul devpaul closed this as completed Feb 14, 2017
@vrozaev
Copy link
Contributor Author

vrozaev commented Feb 15, 2017

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
Copy link
Contributor

devpaul commented Feb 16, 2017

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
Copy link
Contributor Author

vrozaev commented Feb 16, 2017

@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
Copy link
Contributor

devpaul commented Feb 16, 2017

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants