search

aurelia / bundler

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

local package definition missing from bundle when using jspm 0.17.0-beta.40 #153

Closed AdamWillden closed 7 years ago

AdamWillden commented 7 years ago

I'm submitting a bug report or I'm submitting a feature request I'm not sure.

Please tell us about your environment:

Current behavior: Given a jspm.config.js like the following:

SystemJS.config({
  browserConfig: {
    "baseURL": "/dist",
    "paths": {
      "*": "dist/*",
      "github:": "/jspm_packages/github/",
      "npm:": "/jspm_packages/npm/",
      "yw-event-viewer/": ""
    }
  },
  devConfig: {
    // etc
  },
  transpiler: false,
  packages: {
    "yw-event-viewer": {
      "main": "main.js",
      "format": "amd"
    }
  }
});

SystemJS.config({
  packageConfigPaths: [
    "npm:@*/*.json",
    "npm:*.json",
    "github:*/*.json"
  ],
  map: {
    "aurelia-animator-css": "npm:aurelia-animator-css@1.0.1",
    "aurelia-api": "npm:aurelia-api@3.1.1",
    //etc
  },
  packages: {
    "npm:aurelia-animator-css@1.0.1": {
      "map": {
        "aurelia-metadata": "npm:aurelia-metadata@1.0.3",
        "aurelia-pal": "npm:aurelia-pal@1.3.0",
        "aurelia-templating": "npm:aurelia-templating@1.2.0"
      }
    },
    "npm:aurelia-api@3.1.1": {
      "map": {
        //etc
      }
    }
  }
});

While the bundle task works without any errors the resultant file is missing one section:

  packages: {
    // this is the local package definition
    "yw-event-viewer": {
      "main": "main.js",
      "format": "amd"
    }
  }

I suspect (guess) this might be because the systemjs-builder dependency is old (0.15->0.16) and rejects the local package definition as invalid. I tried cloing this repo but I can't get anything to build and so couldn't test that theory.

Another note to make: jspm 0.17 appears to separate the jspm.config.js file into two parts (i.e. two SystemJS.config calls) as you can see above. The I've checked the bundler code and the reader doesn't take into consideration that a single file could have multiple calls although I don't think this is an issue here, A single SystemJS.config call will work so long as this missing package entry is included (I've manually included the missing entry and it works).

Expected/desired behavior: The local package definition is included in the bundle so that the bundler functions for jspm 0.17 projects. Especially now that loader-default is compatible (https://github.com/aurelia/loader-default/commit/a4607b9)

/cc @ahmedshuhel

AdamWillden commented 7 years ago

Please excuse the ping ahmed, I'm in a rather desperate situation (https://github.com/jspm/jspm-cli/issues/2252) so am exploring all options.

EisenbergEffect commented 7 years ago

@ahmedshuhel Can you take a look at this?

ahmedshuhel commented 7 years ago

@AdamWillden @EisenbergEffect Will look into it tonight.

AdamWillden commented 7 years ago

@EisenbergEffect @ahmedshuhel I've just realised I completely missed whats happening and where the main bug with bundling lies and it's not the dependency version.

See this section of code in the config-serializer with special note to cfg[key] = systemCfg[key]; given the new config has two calls to System.config and two packages entries we end up overwriting the first cfg.packages entry. We should be merging them or handling them independently.

ahmedshuhel commented 7 years ago

@AdamWillden What's your bundle config looks like? It would be great if you could point me to a github repo with minimum code that reproduces this bug. However, I am trying to setup it up with skeleton-navigation and see if I can reproduce it. Thanks.

AdamWillden commented 7 years ago

@ahmedshuhel I should be able to sort you out a repo and share a link on gitter but won't be till tonight at best (0830 UK time currently)

ahmedshuhel commented 7 years ago

@AdamWillden Sure. That would be great.

AdamWillden commented 7 years ago

@ahmedshuhel I tried doing this last night but came up against unrelated issues post-upgrade. I'm working on it and you'll have it as soon as I can (home-life permitting)

AdamWillden commented 7 years ago

@ahmedshuhel I've just linked you a repro on gitter

npm install (this will do jspm install) gulp build gulp bundle

Notes:

  1. In bundles.js having the depCache option enabled causes me issues on our CI server (if it caused issues locally I didn't notice). I'm having to run with depCache disabled
  2. gulp unbundle will need fixing once bundle is fixed
  3. I had to update the export-release task in the skeleton as the jspm.normalize function seemingly no longer returns the base path (e.g. 'jspm_packages\npm\font-awesome@4.7.0') but the main

Any issues let me know and I'll be happy to help! The code I've linked is part of our internal company library, I'm pretty sure this skeleton doen't reference anything on our NPM enterprise server so you should be okay running npm install

ahmedshuhel commented 7 years ago

:+1:

ahmedshuhel commented 7 years ago

@AdamWillden Pushed a fix. Give it a try and let us know. Install with npm i aurelia-bundler@beta -S

AdamWillden commented 7 years ago

@ahmedshuhel it still seems to be removing the same local package definition for me (tried with npm i ... --save-dev also)

Could this be a possibility?

the systemjs-builder dependency is old (0.15->0.16) and rejects the local package definition as invalid

AdamWillden commented 7 years ago

It would be much nicer if the bundles section were simply added to one of the existing config calls rather than it rewriting the entire file with a new single config call.

Though I guess that may be at odds with the ability to pass multiple config files?

AdamWillden commented 7 years ago

@ahmedshuhel I tried beta.3 and that worked to keep the required entry! :-) When i did the install the other day I only got beta.2

I stand by my previous comment with regards to not rewriting the entire config with a single call, it'd be much nicer.

AdamWillden commented 7 years ago

@ahmedshuhel yeah if I gulp bundle (which makes it a single config call), gulp unbundle (maintains the single config call) then jspm install some-package the file reverts to having two calls again. Which means the file is never consistent. Not a big deal but a bit 'untidy'. Feel free to keep this closed as the root issue is fixed but perhaps we should open an issue for a future enhancement?

ahmedshuhel commented 7 years ago

@AdamWillden We are aware of that problem (bundler collapsing it into a single SystemJS.config({...}) call). Let's create a separate improvement ticket for that.

AdamWillden commented 7 years ago

Cheers @ahmedshuhel. Thank you very much for your work on this!