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

Can't make multi-module build setup to work #805

Closed
kirill-konshin opened this issue Apr 24, 2015 · 28 comments
Closed

Can't make multi-module build setup to work #805

kirill-konshin opened this issue Apr 24, 2015 · 28 comments

Comments

@kirill-konshin
Copy link

I have the following directory structure:

\_ src
   \_ bower_components
   \_ app
   |  \_ lib
   |      \_ build
   |      \_ build_temp
   |          \_ dir-with-stuff
   |          \_ main.js
   |          \_ main_en-US.js
   \_ require-config.js

I am trying to build files main.js and main_en-US.js (including stuff from bower_components and dir-with-stuff) and put built versions in app/lib/build (should be two files). Files share some dependencies, but they should be included in both.

process.cwd() is src/app/lib since build scripts are located there, because of that we have to go 2 levels up for config and then descend back to build and build_temp.

My current r.js config:

{
    "mainConfigFile": "../../require-config.js",

    "dir": "build",
    "appDir": "app/lib/build_temp",

    "modules": [
        {"name": "app/lib/build_temp/main"},
        {"name": "app/lib/build_temp/main_en-US"}
    ],

    "skipDirOptimize": true,
    "allowSourceOverwrites": true,
    "findNestedDependencies": true,
    "optimize": "none",
    "generateSourceMaps": true
}

I get the following error:

Error: Module ID 'app/lib/build_temp/main' has a source path that is same as
       output path: /Users/xxx/web/src/app/lib/build_temp/main.js. Stopping, config is malformed.
       at /Users/xxx/web/src/app/node_modules/grunt-contrib-requirejs/node_modules/requirejs/bin/r.js:27128:39
@kirill-konshin kirill-konshin changed the title Can't make multi-module setup to work Can't make multi-module build setup to work Apr 24, 2015
@kirill-konshin
Copy link
Author

If I'll use just main as module name, I'll get

Error: ERROR: module path does not exist: /Users/xxx/web/src/main.js for module named: main.
Path is relative to: /Users/xxx/web/src/app/modules/lib

@kirill-konshin
Copy link
Author

As discussed in asciidisco/grunt-requirejs#87 I've tested with 2.1.6 -- worked. But, the RJS has put optimized versions in the same place where they were (appDir), not in the dir as expected. So basically this is the problem, dir is not taken into account at all.

@kirill-konshin
Copy link
Author

The only file that is created in dir is build.txt... This looks very weird.

@jrburke
Copy link
Member

jrburke commented May 2, 2015

This part looks a bit odd to me:

    "modules": [
        {"name": "app/lib/build_temp/main"},
        {"name": "app/lib/build_temp/main_en-US"}
    ],

I would have expected the baseUrl to be set to app/lib/build_temp and that this section would look like this:

"modules": [
    {"name": "main"},
    {"name": "main_en-US"}
],

as the "modules" section is module ID based, not path based, and and module IDs that need to be injected in the built file will be based on the module IDs used here and how they relate to baseUrl or paths settings to find the actual files.

@kirill-konshin
Copy link
Author

I can't change the baseUrl because require config has lots of references to bower components, which also are used for the build.

@jrburke
Copy link
Member

jrburke commented May 3, 2015

Ah, ok, in that case you could use paths config to set up the paths for 'main' and 'main_en-US'.

@kirill-konshin
Copy link
Author

I tried, it went even worse, because the file React.js which is located right near main.js cannot be found, RJS looks for it in the baseUrl for some reason...

@kirill-konshin
Copy link
Author

As I said earlier, in 2.1.6

"modules": [
        {"name": "app/lib/build_temp/main"},
        {"name": "app/lib/build_temp/main_en-US"}
    ],

works well, but, I don't know why, the dir is completely ignored, RJS puts all the files back to their original location, overwriting originals.

@jrburke
Copy link
Member

jrburke commented May 6, 2015

I think the main issue is having the build output directory inside the area that is considered as part of the src area -- it would be best for the output to be outside that directory.

If you want it to be inside the build_temp directory, which seems like is true since "allowSourceOverwrites" is set to true, then typically all the app resources will be in that directory, or the config will be such that the baseUrl is inside that directory.

I am guessing that you have some sort of pre build process that generates build_temp, and you do not want to copy over the src/bower_components to the build_temp directory in that case, and it is complicated by wanting baseUrl to be src/bower_components.

In that case, I would likely put all the "app" stuff (the stuff that is not in src/bower_components) in an "app" dir inside build_temp, and then set a paths config just for the "app" prefix, and the module layer IDs would be "app/main" and "app/main_en-US", and then those modules would use relative IDs for modules that are in dir-with-stuff (with dir-with-stuff being inside the app directory), so like "./dir-with-stuff/react" if it is in there.

@kirill-konshin
Copy link
Author

Build (out) and build_temp (src) are on the same level.

You're right, build_temp is used to store ngAnnotate-pre-processed files.

All I need is: take main* files in build_temp, analyze their local and bower dependencies, concatenate everything in one file per each main*, put those files in build dir. Done. Very simple task... No extra copy of anything, build should have just built versions of main* files.

allowSourceOverwrites did not help at all, it was simply ignored.

@jrburke
Copy link
Member

jrburke commented May 6, 2015

Ah, if you want to just generate a build/main.js and a build/main_en-US.js, and do not need r.js to do any other optimizations, then you can do two single file optimization runs to do that. So, don't use "dir" and "appDir", just use an "out" and a "baseUrl". One run would be for an out="build/main.js" and the other for out="build/main_en-US.js".

@kirill-konshin
Copy link
Author

I tried it before :) this was the first implementation, but, taking into account that some of the modules are LESS files, some are JSX (loaded via plugins), build time grows a lot (main and main_en-US share 90% of the codebase, except localization strings and templates).

So I decided to run multi-module setup, so that modules will be built only once and then assembled in a proper way.

@kirill-konshin
Copy link
Author

It would be great if single-file optimization could take an array of files as name and an array of out...

@jrburke
Copy link
Member

jrburke commented May 6, 2015

That is the extent of the single file support in r.js. If you want to do more custom things in a build process, amodro-trace might be worth considering. For example, if you wanted to save a bunch of the file contents in memory and feed those in memory contents to to amodro-trace, that might save some time.

Going to close for now as the single file builds are the way to go for this kind of project setup, but no plans to add more options for that pathway, with the guidance that amodro-trace should be used for more custom build setups. Feel free to continue discussion here though.

@jrburke jrburke closed this as completed May 6, 2015
@kirill-konshin
Copy link
Author

I disagree with you about your decision to close because multi-module setup still does not work as expected. As I said, with 2.1.6 my setup ignores the dir and places everything in it's original place. Newer versions do the same but they that error message, which could not be overridden with allowSourceOverwrites.

@kirill-konshin
Copy link
Author

Important remark - 2.1.6 works, it builds main* files, so the whole setup is fine, except that it does not place them properly. This is the essence of the discussion.

@jrburke
Copy link
Member

jrburke commented May 6, 2015

For me to debug, I will need a sample project to try. Looking at the fixed issues in 2.1.17 the one that seems the closest related might be #796, but it does not seem related to the layout described above.

@jrburke jrburke reopened this May 6, 2015
@kirill-konshin
Copy link
Author

https://github.com/kirill-konshin/rjs-build-test

Clone, do npm install, then run build.sh:

Added main file build_temp/main.js
Added main file build_temp/main_en-US.js
{ [Error: Error: Module ID 'main' has a source path that is same as output path: /Users/xxx/Sites/_contrib/rjs-build-test/src/app/modules/lib/build_temp/main.js. Stopping, config is malformed.
    at /Users/xxx/Sites/_contrib/rjs-build-test/node_modules/requirejs/bin/r.js:27126:39
]

@kirill-konshin
Copy link
Author

Any updates?

@jrburke
Copy link
Member

jrburke commented May 10, 2015

Sorry, I misread the version numbers before, I thought you were referring to 2.1.16 instead of 2.1.6.

The behavior did change since 2.1.6 because people were overwriting their source with bad configs, and source loss is a serious enough bug that I clamped down on allowing source overwrites.

I spent about 2 hours trying to get something to work, but this setup is really outside the usual uses of a single JS or whole project optimization, and relies on writing/reusing source areas in ways I am not comfortable trying to support.

I appreciate this approach is to try to gain some speed, but I do not recommend it. While the behavior changed since 2.1.6, as mentioned, the source loss bugs people where encountering to me qualified the change as necessary. So I am going to close this, as I do not plan to add ways to make this easier,

I tried to get something that would work under 2.1.17, and did these changes:

diff --git a/src/app/modules/lib/build.js b/src/app/modules/lib/build.js
index 6bd61ed..d20244f 100644
--- a/src/app/modules/lib/build.js
+++ b/src/app/modules/lib/build.js
@@ -7,7 +7,7 @@
         src = 'src',
         mainConfigFile = '../../../requireConfig.js',
         root = path.dirname(mainConfigFile),
-        pathTemp = path.relative(root, path.join(process.cwd(), temp)),
+        pathTemp = path.join(__dirname, 'build_temp'), //path.relative(root, path.join(process.cwd(), temp)),
         pathBuild = path.relative(root, path.join(process.cwd(), build)),
         pathCWD = path.relative(root, path.join(process.cwd()));

@@ -26,16 +26,18 @@
             'excludedComponent'
         ],
         exclude = defaultExlude,
-        files = ['build_temp/main.js', 'build_temp/main_en-US.js'];
+        shareFiles = ['main.js', 'main_en-US.js', 'share.js'];
+        files = ['main.js', 'main_en-US.js'];


     requirejs.optimize({
+        baseUrl: '.',
         mainConfigFile: mainConfigFile,
-        dir: build, // relative to CWD?
-        paths: files.reduce(function(r, file) {
+        dir: path.join(__dirname, '..', 'build'), //build, // relative to CWD?
+        paths: shareFiles.reduce(function(r, file) {

             file = file.replace('.js', '');
-            r[path.basename(file)] = path.join(pathCWD, file);
+            r[path.basename(file)] = path.join(pathTemp, file);

             console.log(r);

It was not enough though, get a "no such file or directory '/Users/jr/git/temp/rjs-build-test/src/app/modules/build/bower_components/component/index.js'" error, so not a complete answer.

@jrburke jrburke closed this as completed May 10, 2015
@kirill-konshin
Copy link
Author

I was working under 2.1.16, the bug (overwrite of original files, ignore of dir option) can be observed there, 2.1.6 has the same bug, but does not have error message (I know the history, in fact, it's a good decision to prevent overwrites).

What I've tried to do with RJS is similar to multiple entry items in Webpack. It allows very flexible configuration what goes where :)

@kirill-konshin
Copy link
Author

It was not enough though, get a "no such file or directory '/Users/jr/git/temp/rjs-build-test/src/app/modules/build/bower_components/component/index.js'" error, so not a complete answer.

No wonder why, you've changed baseUrl but path to bower component in requireConfig.js is relative to requireConfig.js so it can't be found anymore.

@kirill-konshin
Copy link
Author

The biggest question which is still not answered is "Why dir is ignored?"

@jrburke
Copy link
Member

jrburke commented May 13, 2015

dir is ignored because when the final paths are resolved, the paths are outside of the the "dir" directory, so r.js does not know how to relativize the paths to be inside the dir directory.

The baseUrl of '.' helps it know that the source area baseUrl matches to the "dir" + "." for the output baseUrl value.

The optimizer tries to figure out how the "dir" output area should look based on removing the common parts of the absolute path structures compared to the source area. It does this internally by trying to match the path prefixes (absolute path parts at the start of the path string). This assumes that the output area is more like a sibling to the source area, in order for the path prefixes to match. This example is tricky because the build area is nested inside the source area.

The path matching for generating output relative paths could very well be a flawed mechanism inside r.js, but it is hard for me to get traction on it given that the example falls out of what I normally recommend. Unfortunately I just do not have enough time to spend for those cases.

@kirill-konshin
Copy link
Author

I think the implementation should be much-much simpler: copy all the stuff from appDir to dir, fix relative paths (relative to baseUrl) from appDir to dir, optimize, cleanup, done. No magic.

@kirill-konshin
Copy link
Author

In fact, what I was trying to do is only to optimize some main*.js files and place them in dir, I don't even need to copy anything, but RJS does not allow to use dir w/o appDir...

@jrburke
Copy link
Member

jrburke commented May 26, 2015

The path matching was part of the "fix relative paths" in your flow mentioned above. The difficulty is locating the dir inside the appDir. There is likely a way I can do that step better, but to do the initial copies, full paths seemed necessary to copy things to dir, including paths config that may be outside of the appDir.

@kirill-konshin
Copy link
Author

I think that the mechanism is overcomplicated if it fails on the simple task... the task that is covered is actually times harder than what we're talking about in this topic...

I was thinking about using Webpack instead of RJS, but for Webpack it would be better to leave only files w/o plugin prefixes and let Webpack to decide which loader to use for particular file since loaders naming may differ from RequireJS plugins. This is why I raised this question in a topic #812.

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