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

systemjs: getting "unexpected token <" when loading a lib from a separate bundle #676

Open
Thanood opened this issue Jul 11, 2017 · 13 comments · Fixed by #679
Open

systemjs: getting "unexpected token <" when loading a lib from a separate bundle #676

Thanood opened this issue Jul 11, 2017 · 13 comments · Fixed by #679

Comments

@Thanood
Copy link
Contributor

Thanood commented Jul 11, 2017

I'm submitting a bug report

  • Library Version:
    0.30.1

Please tell us about your environment:

  • Operating System:
    Windows 10

  • Node Version:
    7.10.0

  • NPM Version:
    4.5.0

  • Browser:
    all

  • Language:
    TypeScript 2.3.4

Current behavior:

I'm trying to use a library from a separate bundle (so a third one in addition to app-bundle and vendor-bundle).
This approach works flawlessly when using the RequireJS loader.

The bundle contains materialize-css and aurelia-materialize-bridge.

aurelia-materialize-bridge exposes an Aurelia plugin which is used like this (in main.js):

aurelia.use.plugin('aurelia-materialize-bridge', b => b.useAll());

There are no other direct references (maybe that plays a role, I don't know).

This is my bundle config:

      {
        "name": "materialize-bundle.js",
        "dependencies":[
          "jquery",
          {
            "name": "materialize-css",
            "path": "../node_modules/materialize-css/dist",
            "main": "js/materialize.amd",
            "deps": [
              "jquery"
            ],
            "resources": [
              "css/materialize.css"
            ],
            "exports": "Materialize"
          },
          {
            "name": "aurelia-materialize-bridge",
            "path": "../node_modules/aurelia-materialize-bridge/dist/amd",
            "main": "index",
            "deps": [
              "jquery"
            ],
            "resources": [
              "**/*.{css,html}",
              "index.js"
            ]
          }
        ]
      }

The app build just fine but when I load the app, I'm getting an error as soon as the plugin is loaded:

DEBUG [aurelia] Loading plugin aurelia-materialize-bridge.
vendor-bundle.js:1395 Unhandled rejection Error: Unexpected token <
  Evaluating http://localhost:9001/aurelia-materialize-bridge
  Loading http://localhost:9001/aurelia-materialize-bridge

The loader is trying to load this path http://localhost:9001/aurelia-materialize-bridge which does not exist. As a consequence, the actual response is the index.html file which the loader can't evaluate (of course). Thus the error unexpected token <. 😃

This can be solved by placing the configuration inside vendor-bundle.

Expected/desired behavior:

  • What is the expected behavior?

I would expect Aurelia to load the above mentioned plugin from the materialize-bundle configured in aurelia.json.

  • What is the motivation / use case for changing the behavior?

I want to use a third bundle for a rather big plugin.

Also maybe I'm lazy and I'd like to keep the same docs for aurelia-materialize-bridge when using the CLI with SystemJS instead of RequireJS. 😏

Just kidding, I often split large portions into separate bundles to have a little more control.

@Thanood
Copy link
Contributor Author

Thanood commented Jul 11, 2017

Additional note:
I've read #659 but in my case, there are no sourcemaps and the issue can be resolved by moving the configuration into vendor-bundle.

@Thanood
Copy link
Contributor Author

Thanood commented Jul 11, 2017

Reproduction repo: https://github.com/Thanood/md-todo-systemjs

The sass files are not loaded, I've chosen the wrong CSS processor. Please ignore that. 😃

@JeroenVinke
Copy link
Collaborator

@simonfox do you have time to look into this one?

@simonfox
Copy link
Contributor

@JeroenVinke sure, might be a couple of days away though

@Thanood thank you for the detailed issue with repro 👍

@simonfox
Copy link
Contributor

I've had a quick look at this, the SystemJS config for the materialize bundle is missing an entry for aurelia-materialize-bridge because that isn't actually a file that gets traced. The wrapper is generated correctly
i.e. the following definition for aurelia-materialize-bridge is added to the bundle itself

define('aurelia-materialize-bridge', ['aurelia-materialize-bridge/index'], function (main) { return main; });

Just need to ensure SystemJS config has the package name included in the bundle definition (a long with the traced sources).

I will put a PR together later today or tomorrow.

@simonfox
Copy link
Contributor

It works in the vendor bundle because that is loaded via a script tag so the wrapper module is evaluated before the attempt to load the plugin.

simonfox added a commit to simonfox/cli that referenced this issue Jul 16, 2017
`Bundle.getBundledModuleIds` was only returning the ids of the traced files in the case of a package dependency. This change modifies `DependencyInclusion` to include the actual package name in the module ids for the dependency.

closes aurelia#676
@Thanood
Copy link
Contributor Author

Thanood commented Jul 16, 2017

Thanks for looking at this. 😃 👍

JeroenVinke added a commit that referenced this issue Jul 18, 2017
fix(systemjs-bundling): include dependency name in bundle config
@JeroenVinke JeroenVinke reopened this Jul 18, 2017
@JeroenVinke
Copy link
Collaborator

I tried out this fix and it does solve the error mentioned in this issue, but I got another one: #679 (comment).

@Thanood
Copy link
Contributor Author

Thanood commented Jul 18, 2017

Shouldn't this

define('aurelia-materialize-bridge', ['aurelia-materialize-bridge/index'], function (main) { return main; });

rather be something like

define('aurelia-materialize-bridge', ['aurelia-materialize-bridge/index', 'jquery'], function (main) { return main; });

instead when deps are configured like that:

          {
            "name": "aurelia-materialize-bridge",
            "path": "../node_modules/aurelia-materialize-bridge/dist/amd",
            "main": "index",
            "deps": [
              "jquery"
            ],
            "resources": [
              "**/*.{css,html}",
              "index.js"
            ]
          }

AFAIK the error message @JeroenVinke gets in #679 (comment) (Module not already loaded loading "jquery" as http://localhost:9000/jquery.) is the same when trying to synchronously load a requirejs module which was not loaded before. Or in other words: it's probably not pulled as a dependency. 😃

@simonfox
Copy link
Contributor

@JeroenVinke @Thanood this is a hairy one! The problem is in materialize-css. We are right on the edge of amodro/SystemJS compatibility here I think...

I'm not sure the requirejs opitmizer build is appropriate in this instance. Using a package definition similar to this for materialize-css gets you past that error, but you then get a 404 from an attempt to load scripts/picker.js

I tried working backwards from from a jspm/SystemJS project with materialize-css working as expected. But even then, the jspm install adds a 'format global' hint to the top of the script. That is not something that amodro knows about so causes a multiple global exports warning. dist/js/materialize.js has more than one anonymous define. May be a built file from another build system like, Ender. Skipping normalization.

I also looked at a systemjs-builder build of materialize-css, similar to the r.js mentioned above but that is still going to cause problems with the trace.

Any ideas?

@simonfox
Copy link
Contributor

Maybe @guybedford could give us some hints?

@Thanood
Copy link
Contributor Author

Thanood commented Jul 19, 2017

Hmm, aurelia-materialize-bridge contains two script files which convert Materialize to an AMD module. This should work with amodro. The script files are materialize-css.js and rbuild.js.

It's used like this if the files are in project root:
./node_modules/.bin/r.js -o rbuild.js

The result is this module definition: https://gist.github.com/Thanood/e37e239a7d90a1dc3c2789d6f650116e

It's still exposing globals. Materialize depends on it, unfortunately.
But that should in theory get around that nasty picker error.

Maybe even prepend Materialize, just how it's done with bluebird. Would that work?

@simonfox
Copy link
Contributor

simonfox commented Jul 19, 2017

@Thanood yeah that is the build that is causing the Module not already loaded loading error that @JeroenVinke raised. That error comes from here in SystemJS which suggests commonjs require.

Adding

"packages": {
  "materialize-bundle": {
    "format": "cjs",
  }
}

to SystemJS config changes the error to the 404 for picker.js

I did try prepend also, but didn't manage to get that to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants