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

Multi-module builds fail with ENOENT when root module is supplied via rawText #919

Closed
rjgotten opened this issue Jun 20, 2016 · 3 comments
Closed
Milestone

Comments

@rjgotten
Copy link

rjgotten commented Jun 20, 2016

Multi-module builds using the modules option currently fail with a hard node ENOENT error when the root module is specified via the rawText option instead of a normal file.

For some third-party integration scenarios, this can be quite common. Part of such a project may need to build into a standalone bundle meant for consumption on other sites as a 'widget', where the loader mechanic should be encapsulated, e.g., with almond. As part of those builds you may want to use a generated non-module wrapper.

You could use the create:true flag for this, but that would also write out a define() for the generated name, so the proper method is to use rawText with empty contents for the root module; to include the almond loader followed by the actual root module; to include a require call for the root module; and finally to ensure a wrapping IIFE is added and no define call is inserted for the almond loader itself.

E.g.

({
  // ...
  rawText : { "integration" : "" },
  modules : [{
    // ...
  }, {
    name          : "integration",
    include       : [ "almond", "app/integration/main" ],
    insertRequire : [ "app/integration/main" ],
    override      : {
      skipModuleInsertion : true,
      wrap                : true
    }
  }]
})

This causes the build to crash with an ENOENT error for a missing file integration.js. This problem originates from https://github.com/requirejs/r.js/blob/2.2.0/dist/r.js#L25307-L25309 where the following code is run while copying over the module files to the build directory:

if (!module.create) {
  file.copyFile(module._sourcePath, module._buildPath);
}

This treats the create flag as the sole discriminator for whether the root module should be copied over to the build path. If you use rawText to supply the root module, there will be no file to copy and attempting to do so leads to aforementioned ENOENT error from node's built-in fs module.

This check should probably be expanded to verify whether the module exists on disk before attempting to copy, i.e. ;

if (!module.create && file.exists(module._sourcePath)) {
  file.copyFile(module._sourcePath, module._buildPath);
}
@rjgotten rjgotten changed the title Multi-module builds where root module is supplied via rawText fail with ENOENT Multi-module builds fail with ENOENT when root module is supplied via rawText Jun 20, 2016
@jrburke
Copy link
Member

jrburke commented Jun 21, 2016

A thought on the root requirement: if there is a build being generated with almond in it, and it wants to be a fully encapsulated JS file with all its dependencies inside and IIFE, that sounds more like a single JS file build.

Using a whole project build with just one layer specified as an almond build, does not sound like it the best approach since those kinds of self-encapsulated almond builds cannot dynamically load or take advantage of other multi-module build layer tracing for exclusions.

If you needed to generate a few encapsulated almond builds and want to share a common config, this gist shows how that could be done.

Does that address the core need?

Closing as a discussion ticket, but feel free to continue discussion here, and we can reopen if the use case is further refined.

@jrburke jrburke closed this as completed Jun 21, 2016
@rjgotten
Copy link
Author

rjgotten commented Jun 21, 2016

if there is a build being generated with almond in it, and it wants to be a fully encapsulated JS file with all its dependencies inside and IIFE, that sounds more like a single JS file build.

Consider multi-layer builds where only one of the layers is being set up to load standalone through almond for third-party integration, and other layers are part of the first-party multi-page application. E.g. the integration code is used to deploy some kind of widget that links back to functionality of the main application. That's the use-case where you'd want one unified build with shared dependencies.

Sure; you could create separate multi-layer and single-file builds and then wrap a script around them that uses a part of shared configuration and kicks off both builds to ensure their results are in-sync. But that really feels like a hack, more than anything else.

If you needed to generate a few encapsulated almond builds and want to share a common config, this gist shows how that could be done.

Does that address the core need?

That type of scripted solution means custom build tooling which doesn't fit into the regular pattern of calling r.js with a config file from a commandline. While it fits the core need, it does so in a round-about way that may not integrate well with existing build chains. (It is, for instance, quite a pain to set up in MSBuild environments on TFS build servers.)


Separate of the discussion whether this scenario is good practice or not; it's definitely a bad practice to attempt to copy files without checking for their existence. So either way, this is a bug that should still be fixed.

@jrburke jrburke added this to the 2.2.1 milestone Jun 21, 2016
@jrburke
Copy link
Member

jrburke commented Jun 21, 2016

I do prefer to keep an error when the config thinks it can copy a file but it does not exist, this usually means the config is malformed and it is better to fail early with an error vs silently fail but not have the build include something that was wanted. It may be worth adding a check for a rawText entry though and not do a copy in that case.

I still think that the purposes of the builds are separate enough that trying to wedge in a single file JS build with the multilayer build is not ideal, the programmatic build script is a better overall approach. The r.js optimizer's node module API exists for cases that do not fit into the two standard approaches supported by the optimizer.

I will leave this open for consideration in 2.2.1, with perhaps a rawText exception for the copy, but will need to study if that affects other code, if the scope of the change starts to get muddy. I will not have steady access to internet for a few weeks though, may take a while to fully close the loop.

@jrburke jrburke reopened this Jun 21, 2016
@jrburke jrburke closed this as completed in 84f4adb Sep 4, 2016
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