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

yeoman init with Require.js fails #251

Closed
addyosmani opened this issue Aug 23, 2012 · 21 comments
Closed

yeoman init with Require.js fails #251

addyosmani opened this issue Aug 23, 2012 · 21 comments
Assignees

Comments

@addyosmani
Copy link
Member

I discovered this while implementing #246.

Basically, if you accept all yeoman init options and run yeoman server, checking the console - you'll notice that it says there is a missing config file for Require.js. Run yeoman build however and everything builds fine, so its a paths issue.

Also interesting: in your index.html file if you set the data-main for Require.js to scripts/main (or whatever, as long as its the full path) you can get server working, but then build fails.

@addyosmani
Copy link
Member Author

More notes from our discussion about this earlier:

  • http://requirejs.org/docs/api.html#jsfiles RequireJS loads all code relative to a baseUrl. The baseUrl is normally set to the same directory as the script used in a data-main attribute for the top level script to load for a page. so that shouldn' t be an issue, the baseUrl at runtime should be ok
  • here we would try to get the baseUrl config from gruntfile and replace the baseUrl value and stripped it from the main resolved file so that you get in build config baseUrl: script/, name: config
  • not sure about the idea though would need tests to be a little bit more precise we would try to implement what requirejs does internally when it founds a data-main file somewhere near: https://github.com/yeoman/yeoman/blob/master/cli/tasks/usemin.js#L225-231 try to setup the baseUrl to "the same directory as the script used in a data-main attribute for the top level script to load for a page"
    and then just reference the name for the module stripped from the baseUrl (base directory)

@oliverturner
Copy link

@addyosmani This has been the case since #206

See #206 (comment)

@addyosmani
Copy link
Member Author

@codedsignal ya. wasn't able to reproduce a few days ago. We need to figure this out this week.

@oliverturner
Copy link

@addyosmani I have no doubt that you will :)

Undying respect to you and the crew for your awesome efforts so far.

@ghost ghost assigned addyosmani Aug 24, 2012
@addyosmani
Copy link
Member Author

@mklabs I've been taking a look at this for the past few hours and I feel we're close to getting it right, or at least understanding whats been breaking.

Opting for a more explicit path in the index.html data-main (e.g scripts/main) and then a complete path (odd, but yes) in the Gruntfile, something like:

    rjs: {
      optimize: 'none',
      name: 'main',
      baseUrl: '/Users/addy/github/test/app/scripts/',
      wrap: true
    },

gives us a yeoman server that executes your modules without issues and yeoman builds correctly.

That's great, but then you try running in dist what build generated and it looks like although the usemin blocks have been processed and created the (revved)amd-app.js script file correctly, it tries to load up scripts/app.js and fails.

This was a little strange to me because I would have thought that these files would all have been combined and minified correctly. I then experimented with toggling the r.js optimize config and when you don't set this to true you do get

0786eab6.app.js     
6c094302.amd-app.js
650f0fa1.main.js    
c045fe6e.plugins.js

type files generated (which is fine).

Unfortunately, this means that we're missing something that links all of these files up together to the application. Ideally, my expectation would be they would all just be compiled and executed as needed on page load.

I remember adding an r.js specific build profile around I/O for us to workaround the usemin/rev issues and wonder if this is that problem showing its head again. Unfortunately, even that option is a bit of a challenge at the moment because of the usemin handle issue I reported a few days ago.

I'll experiment further and see if theres another workaround that can get the finally built output to do what we expect.

@mklabs
Copy link
Contributor

mklabs commented Aug 24, 2012

Strange indeed.

I would have thought that these files would all have been combined and minified correctly.

I would have said the same thing. rjs should output in the final main.js file, all the files combined (minified should probably be left to the min task). It it's not, then we have a problem here.

Having files around that are revved shouldn't be an issue, as they shouldn't be referenced via any <script /> tag but handled as amd modules.

If you can push your test case in some branch (prob in your yeoman fork), I would be happy to try reproduce the error and investigate further with you.

@addyosmani
Copy link
Member Author

@mklabs pushed what I have to the requirejsconfig branch of this repo.

@mklabs
Copy link
Contributor

mklabs commented Aug 25, 2012

Thanks Addy, I'll take a look asap.

@addyosmani
Copy link
Member Author

@mklabs were you able to find out if there was anything else behind this issue?

@mklabs
Copy link
Contributor

mklabs commented Aug 28, 2012

Unfortunately, nope, I didn't have the chance / time to take a look (just yet).

@jsoverson
Copy link

@addyosmani, it looks like usemin is specifying the data-main attribute as the source to be appended instead of the built file, which is why you aren't getting the modules in the built source. The generated index defines the outfile as amd-app.js, not 'scripts/main' (the data-main attribute).

If I change line 230 in usemin.js from
asset += ',' + main[1] + '.js';
to
asset += ',' + output;
I get a successful build (along with changing the rjs config to include name: 'main').

@addyosmani
Copy link
Member Author

@jsoverson Thanks for looking into this!. Does that also result in a successful yeoman server temporary build too? If so, happy to make the changes needed this evening to get that patched.

@jsoverson
Copy link

Yes, dist/ accessed via yeoman server works the way it is expected.

@addyosmani
Copy link
Member Author

@jsoverson if you get a chance could you verify the latest changes landed above work as expected? Everything seems to be fine other than an exception that runs regardless of build type (<WARN> Cannot read property 'stdout' of undefined Use --force to continue. </WARN>).

@sindresorhus are you able to reproduce?

@jsoverson
Copy link

I still need to manually add name : "main" to the rjs config for it to build. That could be a hardcoded fix in the Gruntfile.js template or it could be automatically generated in that section of usemin, but that's where I don't know best practice for Yeoman.

Personally, as an r.js user, I would prefer it to just be part of the template and avoid much internal magic that makes the config potentially more prone to later confusion or conflict.

Maybe an rjs config generator that would parse the project and offer suggestions is something that could fall into the scope of a Yeoman addon. Let me know if that's something that would make sense in Yeoman somewhere.

@addyosmani
Copy link
Member Author

I still need to manually add name : "main" to the rjs config for it to build. That could be a hardcoded fix in the Gruntfile.js template or it could be automatically generated in that section of usemin, but that's where I don't know best practice for Yeoman.

I'll experiment with this further and land the change if its absolutely needed for the build. I think a hardcoded fix in the Gruntfile.js template would make sense here.

@addyosmani
Copy link
Member Author

Maybe an rjs config generator that would parse the project and offer suggestions is something that could fall into the scope of a Yeoman addon.

I also like the idea of this. Lets discuss it further for the next version of the project at some point.

addyosmani added a commit that referenced this issue Aug 29, 2012
@addyosmani
Copy link
Member Author

Going to leave this open for further testing. Once we're confident that yeoman init builds are working fine with the latest changes, I'll need to make them to the generators also relying on AMD/r.js for their config.

@addyosmani
Copy link
Member Author

@jsoverson can you confirm the latest works for you?

@jsoverson
Copy link

Latest works for me

All Y on yeoman init
yeoman server and browse /app, all amd modules load
yeoman build succeeds (though I get errors on compass task and can't find glyphicons)
yeoman server and browsing /dist looks good. amd-app.js is built properly and includes requirejs and all referenced modules.

@addyosmani
Copy link
Member Author

Perfect! Thank you :)

szinya pushed a commit to menthainternet/yeoman that referenced this issue Sep 17, 2014
szinya pushed a commit to menthainternet/yeoman that referenced this issue Sep 17, 2014
addyosmani added a commit that referenced this issue Apr 24, 2015
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

4 participants