Skip to content
This repository has been archived by the owner on Nov 25, 2020. It is now read-only.

Global Resources should be wrapped in PLATFORM.moduleName #347

Closed
Kukks opened this issue Apr 18, 2017 · 35 comments
Closed

Global Resources should be wrapped in PLATFORM.moduleName #347

Kukks opened this issue Apr 18, 2017 · 35 comments

Comments

@Kukks
Copy link

Kukks commented Apr 18, 2017

The new webpack skeleton breaks when using aurelia-authentication with the following error:

bluebird.js:5253 Error: Unable to find module with ID: aurelia-authentication/authFilterValueConverter
    at WebpackLoader.<anonymous> (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8934:35)
    at step (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8783:23)
    at Object.next (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8764:53)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8758:71
    at 150.__awaiter (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8754:12)
    at WebpackLoader.150.WebpackLoader._import (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8899:16)
    at WebpackLoader.<anonymous> (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8999:44)
    at step (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8783:23)
    at Object.next (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8764:53)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8758:71
From previous event:
    at 150.__awaiter (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8754:12)
    at WebpackLoader.150.WebpackLoader.loadModule (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8986:16)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8977:65
    at Array.map (native)
    at WebpackLoader.150.WebpackLoader.loadAllModules (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8977:32)
    at ViewEngine.importViewResources (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:25374:24)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46142:23
From previous event:
    at loadResources (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46133:7)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46210:14
    at next (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46090:30)
    at runTasks (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46096:10)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46376:16
From previous event:
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46375:21
From previous event:
    at FrameworkConfiguration.apply (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:46360:42)
    at Aurelia.start (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:45976:37)
    at Object.<anonymous> (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:51287:50)
    at step (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:51205:23)
    at Object.next (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:51186:53)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:51180:71
From previous event:
    at main.__awaiter (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:51176:12)
    at Object.configure (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:51224:12)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8648:27
From previous event:
    at config (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8643:54)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8679:12
From previous event:
    at bootstrap (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8678:24)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8665:7
From previous event:
    at run (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8660:59)
    at Object.<anonymous> (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8685:16)
    at Object.149 (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:8686:30)
    at __webpack_require__ (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:50:30)
    at Object.225 (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:16037:18)
    at __webpack_require__ (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:50:30)
    at 0.module.exports.ctor.super_ (http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:147:18)
    at http://localhost:8080/app.8d5bf47e4d9d4ef6239b.bundle.js:150:10
@Kukks
Copy link
Author

Kukks commented Apr 18, 2017

A workaround till the platform.modulename is added in the src is to add

    new ModuleDependenciesPlugin({
      "aurelia-authentication": ["./authFilterValueConverter"]
    }),

to the webpack config

@RWOverdijk
Copy link
Member

@Kukks Could you PR a fix?

@Thanood
Copy link

Thanood commented Apr 18, 2017

Hi.. Since I had a similar problem a few days ago I thought I'd chime in and say that this probably won't work for webpack:

  for (let converter of baseConfig.globalValueConverters) {
    frameworkConfig.globalResources(`./${converter}`);
    logger.info(`Add globalResources value-converter: ${converter}`);
  }

https://github.com/SpoonX/aurelia-authentication/blob/master/src/aurelia-authentication.js#L37-L40

It's because webpack analyzes PLATFORM-moduleName() at build time. At that time the string interpolation is not available.
I've had something similar, using [].map(r => PLATFORM.moduleName(r).

Here's a more complete answer on that topic: aurelia/webpack-plugin#107 (comment)

I hope it helps. 😃

@RWOverdijk
Copy link
Member

Yeah, I really don't like webpack. But a lot of people seem to like it so I'm okay with making it work more verbose like we did for aurelia-form

@Thanood
Copy link

Thanood commented Apr 18, 2017

Build resources are not used anymore in webpack-plugin v2..

@Kukks
Copy link
Author

Kukks commented Apr 18, 2017

So I guess the change would have to be replacing the dynamic global resources to a explicitly typed platform.moduleName wrapped one..
or else we can leave it as is and add the modulename function calls at the end just so it triggers the loader's tracing?

I switched from JSPM to CLI to Webpack in just a year and a half...why can't there just be one dead simple loader that scratches the itch? :(

@RWOverdijk
Copy link
Member

I switched from JSPM to CLI to Webpack in just a year and a half...why can't there just be one dead simple loader that scratches the itch? :

@Kukks 👍 I still use jspm with no issues. My laptop is quite powerful so it's not slow for me. But I get that others might have an issue with it.

Anyway, I'm not sure then. @Thanood You seem to know how this works, mind sharing some info on this?

@Thanood
Copy link

Thanood commented Apr 19, 2017

My experience is limited, tbh. 😃
In my own project, I can't seem to load everything correctly, yet.

My knowledge mostly comes from this comment: aurelia/webpack-plugin#107 (comment)
There are several possible approaches, each with its own drawbacks.

@Kukks seems to be correct when he says:

the change would have to be replacing the dynamic global resources to a explicitly typed platform.moduleName wrapped one

globalValueConverters seems to be a static array. Maybe it's worth experimenting a little with that.

Normally, you would do something like frameworkConfig.globalResources(PLATFORM.moduleName(./authFilterValueConverter)); instead of using the dynamic approach. But since webpack analyzes the source at build time you could try to add moduleName here: https://github.com/SpoonX/aurelia-authentication/blob/master/src/baseConfig.js#L167 and construct the array like so:

globalValueConverters = [
  PLATFORM.moduleName('authFilterValueConverter')
];

I'm not sure if this works but when I understood the description correctly in the webpack-plugin issue liked above, then this might work and you'd keep the "dynamic" approach.

But please read the issue comment I've linked since @jods4 provided a very good explanation of what happens.

@Kukks
Copy link
Author

Kukks commented Apr 19, 2017

@Thanood @RWOverdijk Ok will be giving it a go soon then! I think I've managed to figure out how the platform.modulename works after spending a few days converting my work project to the new webpack ts skeleton. It does have some really nice benefits with how simple code splitting with webpack chunks was made!
You'll be looking at https://github.com/aurelia/webpack-plugin/wiki for quite a while trying to figure out wtf is going on..

@RWOverdijk
Copy link
Member

Oh thanks for all the info guys, much appreciated. I'll make sure to read the wiki in the train soon :)

@RWOverdijk
Copy link
Member

I think we can just wrap the plugins with PLATFORM.moduleName. Webpack understand this, and I don't think other managers care.

@ghiscoding
Copy link

Hello, I'm reading this issue which is also a problem that I have and I'm unsure of what the fix or patch is to do, in order to get pass this error thrown in the console. I'm using the latest skeleton-typescript-webpack, what do I need to change in order to get over this open issue?

@RWOverdijk
Copy link
Member

@ghiscoding all occurrences of .plugin need to be wrapped in PLATFORM.moduleName

So, .plugin('aurelia-orm') becomes .plugin(PLATFORM.moduleName('aurelia-orm'))

But that's just because webpack sucks. Other packagers work fine. This would mean PRing the plugins that use this. It's an easy, BC fix.

@ghiscoding
Copy link

@RWOverdijk thanks I already know about the PLATFORM thing. However I'd like to know, is this something to change in your plugin source code or is it in strictly our code? I'm assuming it's a change in your plugin source code since I already had the PLATFORM wrapper and still had the error thrown, so is that correct?

@RWOverdijk
Copy link
Member

@ghiscoding That is correct, yes. I know that aurelia-config needs this. Here, this comment might explain it better

@shawty
Copy link

shawty commented Sep 1, 2017

Hi SO Microsoft have used web pack in all their current Visual Studio 2017 SPA templates including the Aurelia one, and I now find that I'm getting hit by not finding the other modules in au-auth.

I tried adding the work around mentioned above to my webpack config and it just gave me build errors.

How do I fix this?

I'm knee deep in the middle of a new app and I can't go back and change now, and I can't add auth to the app because of this.

Where exactly do I add the web pack part, how do I get it working for now, is there some changes I can make to my installed aurelia-authentication? if yes, then can someone tell me what they are and how?

Given that all the DotNet core 1.4 and 2.0 SPA templates are now web pack from "project->new" there's going to be a lot more folks like me wanting to know how to solve this?

Cheers
Shawty

@RWOverdijk
Copy link
Member

@shawty It's a difficult question to answer. I think the main problem is around the way webpack works with plugin names. It's a giant pita for me because I don't use it.

I could figure it out and solve it but I'm knee-deep in paid projects myself. The next project I get where someone needs me to use webpack I'll solve this, but so far nobody has requested this from me.

Maybe you can take a look? If you're a skilled wielder of webpack that is :)

@ghiscoding
Copy link

@shawty I believe that we are waiting for a fix in the aurelia-authentication files, a few files in the library have to be wrap with the new PLATFORM.moduleName() as @RWOverdijk was describing

@ghiscoding all occurrences of .plugin need to be wrapped in PLATFORM.moduleName
So, .plugin('aurelia-orm') becomes .plugin(PLATFORM.moduleName('aurelia-orm'))
But that's just because webpack sucks. Other packagers work fine. This would mean PRing the plugins that use this. It's an easy, BC fix.

However if you can't wait like me, I actually got the original aurelia-auth library (from Paul V.) working with the patch describe here (sadly this patch doesn't seem to work with aurelia-authentication) implemented in my sample code here. You can take a look at my full project here, a quick note be aware that I replaced MongoDB by RethinkDB (that's just the server side though), the best sample in my project is the client-ts-wp.

@RWOverdijk
Copy link
Member

@ghiscoding Why wait? If people need it, why not add it? That's why it's open source. :)

@ghiscoding
Copy link

ghiscoding commented Sep 1, 2017

@RWOverdijk yeah I know, sorry about that but I am also not a WebPack guru, I would rather leave this to people with more knowledge in touching/fixing your library. I could give it a try, but I'm too scared in affecting other users, unless you have plenty of unit testing in place?

EDIT
I also have to admit that it was my first attempt at using your library, it's a bit different compare to aurelia-auth and because of that, I'm not sure that I'm the good candidate to start modifying your code since I don't have any reference of previously working apps which is also why it scares me to update your library.

@shawty
Copy link

shawty commented Sep 1, 2017

thanks for replying guys, I'm not what I would call a webpack guru either, infact, I'm the type of "I know it's there and it does stuff for me" person when it comes to WP.

Problem is, as I said, MS with all their latest SPA templates for DotNet core have started to use it, not just the Aurelia ones, but Angular, React etc too, couple that with the fact that Rob Eisenberg is back working at MS now, and Steve Sanderson (MS-UK) is maintaining the SPA templates and pushing for Aurelia to be adopted, means that pretty soon Aurelia usage is probably gonna ramp up significantly.

Now I'm happy to have a go at it, in between the paid stuff I'm doing, but at the moment I'm running 4 projects and on writing another 2 books, so time's not really my friend, what Little spare time I do have is taken up helping to run Lidnug.

For the record, I was actually fiddling around with stuff erlier, and I actually got it to work by pre loading the filter as an Aurelia Resource, so that AU already knew where to find it when the full auth lib was called, so the solution may just be a change to the docs on how to implement it (2 lines in the AU use statement vs 1 line) , something like:

import 'isomorphic-fetch';
import { Aurelia, PLATFORM } from 'aurelia-framework';
import authConfig from './auth.config';
import 'bootstrap/dist/css/bootstrap.css';
import 'bootstrap';
declare const IS_DEV_BUILD: boolean; // The value is supplied by Webpack during the build

export function configure(aurelia: Aurelia) {

  aurelia
    .use
    .standardConfiguration()
    .globalResources(PLATFORM.moduleName('aurelia-authentication/authFilterValueConverter'))
    .plugin(PLATFORM.moduleName('aurelia-authentication'), baseConfig => { baseConfig.configure(authConfig) })
    .plugin(PLATFORM.moduleName('aurelia-validation'));

  if (IS_DEV_BUILD) {
    aurelia.use.developmentLogging();
  }

  aurelia.start().then(() => aurelia.setRoot(PLATFORM.moduleName('app/app')));
}

Is working for me right now.

To be fair actually, it wasn't my idea, it was one of the guys I hired who suggested it, and being a Friday night, can of beer in my hand, I brazenly challenged him saying it wouldn't work :-)

Imagine my surprise when it did.

I still need to hook things up to the router and stuff, and check the auth works as expected, but WP seems happy to build on that, and load everything it requires.

Shawty

@ghiscoding
Copy link

@shawty that totally make sense and is the same patch that got me going with aurelia-auth. The only difference is really the name of the plugin to wrap (in aurelia-auth the plugin is called aurelia-auth/auth-filter while in aurelia-authentication it's aurelia-authentication/authFilterValueConverter) that totally make sense.

Thanks for that, I'll probably give a try with aurelia-authentication

@shawty
Copy link

shawty commented Sep 2, 2017

Well I'm no aurelia expert (Although Iv'e been told I am :-) ) but maybe the solution to all of this is to find if there is some hook point in the main authentication library, that can be used to "pre-register" these deps.

Maybe something like:

if(webpack detected)
{
pre-register stuff
}
else
{
dont pre register stuff
}

...
attempt to use stuff
...

To me that seems as if it would then allow the lib to work in both a webpack environment and a non webpack environment.

Potentially if the auth main library follows the same page life cycle as a UI component, then maybe the "activate" function could be used for the purpose. Iv'e not dug into the auth code however so I'm just taking a stab in the dark here

Shawty

@RWOverdijk
Copy link
Member

@shawty I've taken the liberty to edit your comment so the syntax highlighting works.

I know I haven't been the most active maintainer as of late, but that's because I no longer work with Aurelia full time. I can maintain efficiently, just the coding parts are slower :)

If a fix as simple as that works, then great! Aurelia-config uses .plugin(), so that should probably be changed. It uses dynamic module names though, so I'm curious if that would work at all...

@jods4
Copy link

jods4 commented Sep 3, 2017

I have no read everything so I don't know if it's a good solution but for your information:

if you want conditional code based whether the new wepback plugin is used or not, you can check if the global variable AURELIA_WEBPACK_2_0 is defined.

Example in Aurelia framework itself:
`https://github.com/aurelia/bootstrapper/blob/c781a80515d35c9ab2f571138fe916d2bfe47d1f/dist/aurelia-bootstrapper.js#L41

A bonus is that when compiled with webpack and optimized, code behind a if (typeof AURELIA_WEBPACK_2_0 === undefined) condition will be removed from output.

@shawty
Copy link

shawty commented Sep 3, 2017

@jods4 It's probably worth looking at, I may take a peek if I get 5 minutes.

However, we want to keep the check in the output, we don't want WP removing it (At least I don't think anyway) as we need it to remain independent and have a branch for WP and a branch for non WP, which is active at run time, either that or I'm not understanding how WP works.

@shawty
Copy link

shawty commented Sep 3, 2017

@RWOverdijk no worries, edit away :-) and yes I understand where your coming from, that was why I left Knockout Behind......

@ghiscoding
Copy link

@shawty I think what @jods4 means is that you can use that global variable to check if WebPack is used, and if it is then wrap the code with the usual PLATFORM.moduleName() else no wrapper needed. That is to be used in the core code of Aurelia-Authentication.

@shawty
Copy link

shawty commented Sep 3, 2017

Ah...... :-)

@jods4
Copy link

jods4 commented Sep 3, 2017

My comment was in response to #347 (comment) where there's pseudo-code if (webpack detected).

It's just how you can easily and efficiently implement the if (webpack) part.

@ghiscoding
I am not sure why you would wrap with moduleName code when calling webpack and otherwise not.
moduleName is an Aurelia-specific solution that was designed to be usable in all situations (otherwise we could have used webpack-specific concepts)

When bundling with Webpack, all moduleName calls are removed from output. When using another bundler, aurelia-pal actually implements it as moduleName(moduleId) { return moduleId; } so you can keep them in the code without any problem.

@shawty
Copy link

shawty commented Sep 4, 2017

Hi all,

Ok so I had a fiddle with things over the weekend, I kind of made some things work, and I think, there is a possibility I MIGHT be able to do something, it's not going to happen for a little while yet however, as I have things working for now in a way they are needed, and since I'm already on a tight deadline that has to be my priority.

However, in the spirit of code sharing, I need to implent roles in my AU app too, so here's my route auth step (RoleStep.ts), if anyone want's to use it.

import { inject } from 'aurelia-dependency-injection';
import { Redirect } from 'aurelia-router';
import StorageService from "../libs/StorageService";
import { JwtDecode } from "aurelia-plugins-jwt-decode";

@inject(StorageService)
export class RoleStep {

  private storage: StorageService = null;

  constructor(storage: StorageService) {
    this.storage = storage;
  }

  run(routingContext, next) {

    let config = routingContext.config;

    if (config.settings == undefined) {
      return next();
    }
    
    if (config.settings.roles == undefined)
    {
      return next();
    }

    if (config.settings.roles.length === 0) {
      return next();
    }

    let authObject = this.storage.getObject("aurelia_authentication");
    if(authObject == null)
    {
      // there is no aurelia authentication object, so that must mean where not actually logged in
      // all we can do is just drop out to the next... I think anyway - TODO: check id we can do this better
      return next();
    }

    let userRoles = this.getRolesFromToken(authObject.token);
    let pageRoles = config.settings.roles;

    let userRolesContainsPageRoles = userRoles.some(role => pageRoles.includes(role));

    if(!userRolesContainsPageRoles)
    {
      return next.cancel(new Redirect("403"));
    }

    return next();
  }

  private getRolesFromToken(token: string): string[] {
    let roles: string[] = [];

    let decodedToken = JwtDecode.decode(token);
    console.log(decodedToken);

    // In a JWT created by .NET if only one role is defined it's returned as a string, we only get a string array IF there is more than one role
    if (typeof (decodedToken.role) == "string") {
      roles.push(decodedToken.role);
    }
    else {
      decodedToken.role.forEach(role => {
        roles.push(role);
      });
    }

    // Roles will always be returned to the app as an array
    return roles;
  }

}

My storage service is quite simple (I can't give you a copy of that, well not as is anyway... I MAY be able to put my app skeleton on my github page once this project is finished and Iv'e removed the client specific stuff from it), as I was saying, it's quite simple, it just does a JSON.stringify and a JSON.parse and null checks for keys etc, so it's easy enough to replace.

It looks for "settings: { roles: [] } on a route object, and if the roles array is empty, then no access restrictions are applied, if roles[] has any strings in it EG: "roles["admin","user"], then each string is checked against a "role" element in the JWT (I'm using .NET core to generate the JWT, and at the root of the pay load is a role: ["a", "b", "c"] array holding the logged in users roles.

This can obviously be greatly improved, which is why I'm putting at a paste here, rather than as a pull request or anything.

Feel free to use as you see fit, little thanks/credit wouldn't go amiss if you do use it, but I'm not gonna hold you to it :-)

Shawty

@shawty
Copy link

shawty commented Sep 4, 2017

PS: If you wanna find me, just search lidnug on Linked-in, or got to lidnug.org , I help run the user group, so I can generally be found floating around somewhere near by.

@RWOverdijk
Copy link
Member

@shawty feel free to join us on Gitter, too. https://gitter.im/SpoonX/Dev

Also updated your comment again :D For future reference:

```js
// Code here
```

// Code here

@shawty
Copy link

shawty commented Sep 4, 2017

Yea, I keep hitting the "Add Code" button in the editor then pasting my code in where it places the cursor, so Iv'e no idea why it keeps ending up getting crapped up :-)

But yea, edit away... it's all good.

Oh and feel free to add that step to the main aurelia-authentication lib if you wish.

I may come say hello on gitter. I do occasionally pop into the aurelia/validation channel, as I was in there a few months ago trying to work something out, and still get the notifications from it.

Shawty

@doktordirk
Copy link
Contributor

original issue fixec

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

No branches or pull requests

7 participants