aurelia / bundler

A library for bundling JavaScript, HTML and CSS for use with SystemJS.
MIT License
37 stars 25 forks source link

Bug in bundling resources of private npm packages (containing @) #151

Closed genadis closed 7 years ago

genadis commented 7 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior:

adding resource of private npm package to bundle configuration, for example:

[
    @example/my-package,
    @example/my-package/my.resource.css!text   // this line throws the error
]

throws exception: "Error: A version conflict was found among the module names specified in the configuration for..."

The error is thrown because the getFullModuleName does not match correctly private packages.

First the package tries to exact match, so this works for something like @example/my-package, but if you try to add a resource like @example/my-package/my.resource.css!text, exact match wont work and the cleanName function is used to clean the module name.

The result of running the cleanName function on @example/my-package/my.resource.css!text is empty string and no match is found.

To reproduce, from command line:

node -e "console.log('@example/package/my.resource.css!text'.replace(/^.*:/, '').replace(/@.*$/, ''));"

@example/my-package/my.resource.css!text should match @example/my-package module.

bundling of private npm repositories with resources. Consider a repo that contains number of css styles (themes) in addition to the js code. The user of the repo want's to chose the required style for his project and bundle it as part of the application.

ahmedshuhel commented 7 years ago

@genadis I am not sure how a private npm package is different here. It should work either way. Can you create a map in config.js (systemjs config file) for the private package you are using and use that in bundle config and see how that goes?

genadis commented 7 years ago

@ahmedshuhel Thank you for quick response

I already did (actually I'm giving jspm install do the dirty work, but usually check the map: [] of the generated config.js for correctness if there are issues)

Below I've posted the configuration I'm using. But first allow me to point out that I have the same package in the public npm called encapsulated-mdl which works just fine. And the private package when configured with bitbucket worked for me as well. The issue started when I switched to private npm.

For example in my map I have the following:

map: [
"@annoto/mdl": "npm:@annoto/mdl@1.2.6",
 //...
]

@annoto/mdl has no dependencies so it's the only record in the map.

In the project I have a directory for the package:

jspm_packages
   -- npm
      -- @annoto
         -- mdl@1.2.6
           -- material.js
           -- material.cyan-blue.min.css
           -- material.deep_orange-pink.min.css
           -- ....

When the bundle configuration includes only this:

"includes": [
        "@annoto/mdl",
        // ...
]

(I have the package.json of the @annoto/mdl configured correctly to point to material.js as the main)

All is good because the getFullModuleName function of the bundler has an exact match of the module here: Line 189.

When I try adding a css:

"includes": [
        "@annoto/mdl",
        "@annoto/mdl/material.cyan-blue.min.css!text", 
        // ...
]

(By the way before looking into bundel.js I've tried "npm:@annoto/mdl@1.2.6/material.cyan-blue.min.css!text" as well, without success")

I get the mentioned error. As I've pointed out, I believe it is because of the cleanName let cleanName = (n: string) => n.replace(/^.*:/, '').replace(/@.*$/, ''); on Line 188.

I understand it's purpose is to clean the versions of the module which use the @ character, but as a side effect it deletes the private module name as well as it starts with @ returning empty string for all private modules. It should recognize that if @ is the first character it is private module and not a version indicator and not clean it.

Thanks!

genadis commented 7 years ago

@ahmedshuhel @EisenbergEffect is there any update on this? If it's not a bug, and there is issue with the configuration I've posted above I would really appreciate pointers to solving it. Thank you in advance!

It really blocks us from using private npm with aurelia-bundler, for the time being we are using bitbucket.

ahmedshuhel commented 7 years ago

I will look into it and let you know.On Dec 31, 2016 6:57 PM, Genadi notifications@github.com wrote:@ahmedshuhel @EisenbergEffect is there any update on this? If it's not a bug, and there is issue with the configuration I've posted above I would really appreciate pointers to solving it. Thank you in advance! It really blocks us from using private npm with aurelia-bundler, for the time being we are using bitbucket.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

TimBailey-pnk commented 7 years ago

Hi guys, I have this same issue. With my config as....

"@timbailey/control-dashboard",
"@timbailey/control-dashboard/dashboard.html!text"

The message I get is:

Error: A version conflict was found among the module names specified in the configuration for '@timbailey/control-dashboard/dashboard.html!text'. Try including a full module name with a specific
version number or resolve the conflict manually with jspm.
    at getFullModuleName (C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\bundler.js:232:9)
    at C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\bundler.js:55:12
    at Array.map (native)
    at createBuildExpression (C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\bundler.js:54:40)
    at Object.bundle (C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\bundler.js:109:25)
    at _bundle (C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\index.js:78:18)
    at C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\index.js:53:18
    at Array.forEach (native)
    at Object.bundle (C:\Projects\timbailey\Source\myApp\node_modules\aurelia-bundler\dist\index.js:46:24)
    at Gulp.<anonymous> (C:\Projects\timbailey\Source\myApp\build\tasks\bundle.js:13:18)
    at module.exports (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\lib\runTask.js:34:7)
    at Gulp.Orchestrator._runTask (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\index.js:273:3)
    at Gulp.Orchestrator._runStep (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\index.js:214:10)
    at C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\index.js:279:18
    at finish (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\lib\runTask.js:21:8)
    at cb (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\lib\runTask.js:29:3)
    at finish (C:\Projects\timbailey\Source\myApp\node_modules\run-sequence\index.js:60:5)
    at runNextSet (C:\Projects\timbailey\Source\myApp\node_modules\run-sequence\index.js:88:5)
    at Gulp.onTaskEnd (C:\Projects\timbailey\Source\myApp\node_modules\run-sequence\index.js:75:5)
    at emitOne (events.js:101:20)
    at Gulp.emit (events.js:188:7)
    at Gulp.Orchestrator._emitTaskDone (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\index.js:264:8)
    at C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\index.js:275:23
    at finish (C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\lib\runTask.js:21:8)
    at C:\Projects\timbailey\Source\myApp\node_modules\orchestrator\lib\runTask.js:52:4
    at f (C:\Projects\timbailey\Source\myApp\node_modules\end-of-stream\node_modules\once\once.js:17:25)
    at DestroyableTransform.onend (C:\Projects\timbailey\Source\myApp\node_modules\end-of-stream\index.js:31:18)
    at emitNone (events.js:91:20)

As its complaining about "versions" I think that the leading @ is causing the "startswith" version check to trigger.

Thanks

TimBailey-pnk commented 7 years ago

Further to my comment, I think I've resolved with a change to the "cleanName" function....

export function getFullModuleName(moduleName: string, map: any) {
  let cleanName = (n: string) => {

    // strip leading 'registry' prefixes
    let result = n.replace(/^.*:/, '');

    // strip trailing version info
    if (result.charAt(0) === '@') {
      result = '@' + result.substr(1).replace(/@.*$/, '');
    } else {
      result = result.replace(/@.*$/, '');
    }

    return result;
  }
...
...

There might be a neater way to handle this using regexes? I can submit a PR if required.

genadis commented 7 years ago

@ahmedshuhel any updates on this. seems like @TimBailey-pnk posted the solution (Thanks! by the way)

AdamWillden commented 7 years ago

Am I right this issue can now be closed thanks to the linked PR?

ahmedshuhel commented 7 years ago

I believe so.