normalize fails for relative imports with prefixes

[rockymeza]

Hi,

I ran into an issue when trying to use the r.js optmizer with almond. I would have liked to open a pull request, but I'm a little unsure of how where to start with it.

Problem

I'm using relative imports from files that are using loader plugins. Here's what my set up is like:

// main.js
require(['jsx!components/Foo'], function(Foo) { // do stuff });

// components/Foo
define(['jsx!./Bar'], function(Bar) { // do stuff });

// components/Bar
define(/* doesn't matter */);

I get an error because it's trying to use req.toUrl('jsx!jsx!components/Bar'). req.toUrl isn't available obviously, but the module ID is also wrong.

Proposed Solution

I found out that the module ID is being normalized incorrectly, so I modified this line to look like the following and then the problem was solved.

baseParts = baseName && splitPrefix(baseName)[1].split("/"),

I'm not sure what to do next though.

[jrburke]

When using loader plugins with almond, the loader plugin should generate a named define call in the built output because almond does not support dynamic loading, which is something where req.toUrl() can come into play, and if the loader plugin is implementing a loaderPlugin.normalize method, it should be a fairly pass-through result.

So my initial impression is that perhaps a fix in the jsx loader plugin is needed. If you can pass the built output or a test I can try with the output, that would help to identify the source of the problem.

[rockymeza]

I can't share the original, but I have created an example repo that has the same issue:

https://github.com/fusionbox/almond-jsx-bug

Here is the actual application code in the built file:

https://github.com/fusionbox/almond-jsx-bug/blob/master/main-built.js#L16086-L16107

When using loader plugins with almond, the loader plugin should generate a named define call in the built output because almond does not support dynamic loading, which is something where req.toUrl() can come into play, and if the loader plugin is implementing a loaderPlugin.normalize method, it should be a fairly pass-through result.

I'm a little confused, sorry. If I understood correctly, the modules do have names in L16086 and L16091.

But the dependency paths are not normalized, should the jsx-requirejs-plugin be normalizing those?

Thanks!

[mochja]

This also happens with text plugin.

Plugin generates correctly

define('text!src/file', [], function() {
  return 'hello';
});

Problem is that definition which is using this module don't resolve the relative path.

require('text!./file', function(file) { // exception thrown "Dynamic load not allowed: text"
  console.log(file);
}

if you put absolute path to it, it works correctly

require('text!src/file', function(file) {
  console.log(file); // hello
}

If r.js could replace relative paths with absolute path, this would be resolved. Same thing happens with amdclean as well.

[jrburke]

For the require(['text!./file'], function(file) { is that done inside a module's define method? If so, does it use a local require dependency for it? If the answer is no to one of those, then I can see that the relative './file' part will be normalized incorrectly.

[pieceOpiland]

I also seem to be running into this problem with the require-cs plugin.

I've thrown together a jsfiddle containing the built output at: https://jsfiddle.net/bbwvf785/1/

It is unable to resolve the module cs!./myModule. When I substitute absolute paths in for the relative paths, the module is properly resolved. I believe it's because the module it's relative to also contains the plugin prefix. As a result, almond searches for cs!cs!src/myModule similarly to how it looks for jsx!jsx!components/Bar in the original comment.

I tried adding

relName = splitPrefix(relName)[1];

after line 208 and it seems to have resolved the issue in my scenario.

If I can provide any more information, please let me know.

[jrburke]

Sorry it took so long to really address this: the normalization logic was not using the reference ID's resource name, but the full ID, including the plugin prefix part. This was incorrect, requirejs does not do this. I think I just goofed when I ported over some of the requirejs logic a while back.

Fixed in 0.3.3, tagged and published.

[rockymeza]

Thanks James!

On Thu, Sep 1, 2016, 01:11 James Burke notifications@github.com wrote:

Sorry it took so long to really address this: the normalization logic was
not using the reference ID's resource name, but the full ID, including the
plugin prefix part. This was incorrect, requirejs does not do this. I think
I just goofed when I ported over some of the requirejs logic a while back.

Fixed in 0.3.3, tagged and published.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#101 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABVGI-F04BL1fRrH8VkA1_cZt_LJ-H4ks5qll5mgaJpZM4DV3SC
.

-Rocky