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

PLATFORM.moduleName inconsistent behavior #121

Closed
pat841 opened this issue Aug 31, 2017 · 7 comments
Closed

PLATFORM.moduleName inconsistent behavior #121

pat841 opened this issue Aug 31, 2017 · 7 comments
Labels

Comments

@pat841
Copy link

pat841 commented Aug 31, 2017

I'm submitting a bug report

  • Library Version:
    2.0.0-rc.2 but I have also tested against rc.5

Please tell us about your environment:

  • Operating System:
    OSX 10.12.6

  • Node Version:
    7.4.0

  • NPM Version:
    4.0.5

  • JSPM OR Webpack AND Version
    Webpack 3.3.0

  • Browser:
    Chrome 60

  • Language:
    ESNext

Current behavior:

PLATFORM.moduleName() behaves differently when using absolute, relative, and alias paths in different areas.

Expected/desired behavior:

If using a relative path, I expect the relative module to be loaded/used.
If using an absolute path, I expect the absolute module to be loaded/used.
If using an aliased path, I expect the aliased module to be loaded/used.

Example Skeleton:

I cloned the most recent webpack skeleton (Aug 31st) and modified it to provide examples here:
https://github.com/pat841/aurelia-webpack-test

Files of interest:

  • main.js
  • app.js
  • webpack.config.js

main.js:

    /**
     *  FEATURE EXAMPLES
     */

    // Working as expected
    // .feature(PLATFORM.moduleName('test_feature/index'));

    // Not working?
    // .feature(PLATFORM.moduleName('test_feature'));
    // .feature(PLATFORM.moduleName('./test_feature/index'));
    // .feature(PLATFORM.moduleName('./test_feature'));

    /**
     *  PLUGIN EXAMPLES
     */

    // Working as expected
    // .plugin(PLATFORM.moduleName('test_plugin/index'));
    // .plugin(PLATFORM.moduleName('test_plugin'));

    // Not working?
    // .plugin(PLATFORM.moduleName('./test_plugin/index'));
    // .plugin(PLATFORM.moduleName('./test_plugin'));

    /**
     *  ALIAS EXAMPLES
     */

    // Working as expected
    // .feature(PLATFORM.moduleName('test_alias/test_feature/index'));
    // .plugin(PLATFORM.moduleName('test_alias/test_plugin/index'));
    // .plugin(PLATFORM.moduleName('test_alias/test_plugin'));

    // Not working?
    // .feature(PLATFORM.moduleName('test_alias/test_feature'));

app.js:

    config.map([
      // Working as expected
      { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('./sample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('./test/testsample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('./test_feature/testfeaturesample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('./test_plugin/testpluginsample'), title: 'Sample' }

      // Not working?
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/sample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/test/testsample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/test_feature/testfeaturesample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/test_plugin/testpluginsample'), title: 'Sample' }

      // Somehow working?
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('sample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test/testsample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_feature/testfeaturesample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_plugin/testpluginsample'), title: 'Sample' }
    ]);

webpack.config.js:

  resolve: {
    extensions: ['.js'],
    modules: [srcDir, 'node_modules'],
    alias: {
      test_alias: srcDir
    }
  },

As you can see above, PLATFORM.moduleName() acts differently depending on WHERE its used (feature, plugin, route) as well as HOW the path is represented (relative, absolute, or aliased).

Simple Example: Aliased paths are working when using .plugin() or .feature() but not inside route moduleIds.

@jods4
Copy link
Contributor

jods4 commented Aug 31, 2017

The truth is, moduleName does nothing, it is removed from code.

I agree wholeheartedly that this situation is really not optimal but it is caused by how Aurelia works and this predates Webpack by a bunch.

The trick is that the relative module name you put in there is going to be resolved to a full module name twice: once at build-time by webpack; and then once at runtime by aurelia-loader.
Of course they should resolve to the same module otherwise things won't line up properly.

Webpack is straightforward as it always resolves relative to the current file path but Aurelia can be a little unpredictable as to relative to what context it is resolving paths. You noticed correctly that a plugin or feature tend to accept relative ./ paths easily, other places not so much.

This is the reason why I advise people using Webpack to use absolute module names in their app. As you noted, it works consistently everywhere, as there is no normalizing required.

As far as .feature() goes, it's even worse. The old API did append /index implicitly but combined with Webpack it created a huge mess that I won't go into details here (hint: in this situation Aurelia can't tell the difference between a folder and a file, which totally confuses what the resolution context should be).

I am calling it out here: https://github.com/aurelia/webpack-plugin/wiki/Managing-dependencies#platformmodulename
You should always append /index yourself when calling .feature() and using Webpack.

I am open to new ideas but I've thought about this a lot and I see nothing I can do to make it better.

@jods4 jods4 closed this as completed Aug 31, 2017
@jods4 jods4 added the wontfix label Aug 31, 2017
@pat841
Copy link
Author

pat841 commented Sep 1, 2017

@jods4

Yeah my biggest issue is aliased paths dont work when setting moduleId in the route config.

For example, I have an alias "test_alias" that points to src/. I can easily do the following to init my plugin/feature:

    // Working as expected
    // .feature(PLATFORM.moduleName('test_alias/test_feature/index'));
    // .plugin(PLATFORM.moduleName('test_alias/test_plugin/index'));
    // .plugin(PLATFORM.moduleName('test_alias/test_plugin'));

However, if I try to do the same in a route config:

      // Not working?
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/sample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/test/testsample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/test_feature/testfeaturesample'), title: 'Sample' }
      // { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/test_plugin/testpluginsample'), title: 'Sample' }

I have no issue using absolute paths, but I prefer to use an alias so I can namespace my own plugins/features. I just feel it inconsistent to have aliases working when setting a plugin/feature but not anywhere else.

Unless I am doing something wrong?

@jods4
Copy link
Contributor

jods4 commented Sep 2, 2017

are those modules referred to from other places?

The webpack plugin needs to find a normalized name for the module that the runtime can find.
I use a trick where if the module was referred by an absolute name, it's used "as is" as the module name with no attempt at normalization (which is very tricky). This should guarantee that loading the module works at runtime.

In the router situation, can you tell me what is the name of test_alias/sample in the bundle (find out with webpack --display-modules)?
And what name is Aurelia using at runtime when it fails (it's logged in the console)?

Aside: keep in mind that alias config is sometimes useful but it's only a build-time thing.

@pat841
Copy link
Author

pat841 commented Sep 4, 2017

@jods4

I found something interesting. Using this as my route where "test_alias/" points to "src/" which contains "sample.js" and "sample.html":

app.js:

      // Not working?
      { route: ['', 'sample'], name: 'sample', moduleId: PLATFORM.moduleName('test_alias/sample'), title: 'Sample' }

sample.js:

export class Sample {
  //
}

sample.html:

<template>
  INNER SAMPLE
</template>

The webpack module output is:

...
[app] ./src/app.js 2.05 kB {0} [built]
[app.html] ./src/app.html 95 bytes {0} [built]
...
[main] ./src/main.js 7.04 kB {0} [built]
...
[test_alias/sample] ./src/sample.js 230 bytes {0} [built]
...
[sample.html] ./src/sample.html 61 bytes {0} [built]
...

Notice how the absolute path is correctly referencing the js module but NOT the accompanying html view.

This leads to this console error when loading the application:

aurelia-logging-console.js?3f22:47 ERROR [app-router] Error: Unable to find module with ID: test_alias/sample.html

@jods4
Copy link
Contributor

jods4 commented Sep 4, 2017

That's very interesting!

Maybe it's worth opening an issue... but I am not sure how ConventionDependenciesPlugin (the part responsible for including convention-based views in your build) should behave instead.

Currently, it tries to modify (by swapping the extension) the imported file path (your viewmodel), hoping to find a new file (the view) that is included as a dependency of the first.

So far, so good but later we need to determine the correct module id for the view, which is entirely based on file location. Aliases are not taken into account because it's just not working. "Reversing" aliases could lead to several possible solutions and even when there's just one, you can't possibly know if it was used for the viewmodel or not.

Trying to base the view id on the viewmodel id might be another idea but it's tricky. Remember the file path can be changed arbitrarily (e.g. to take views from another folder altogether) and as the viewmodel request could be relative, parts of the manipulated path could be missing from the module request.

If you have a better specification for how ConventionDependenciesPlugin should find views along viewmodel and how it should determine their id, then I'm open to implementing it.

@pat841
Copy link
Author

pat841 commented Sep 5, 2017

@jods4

I created a PR #122 with a fix to PreserveModuleNamePlugin. It works for both absolute and normal alias paths.

@jods4
Copy link
Contributor

jods4 commented Sep 5, 2017

@pat841 I'll have a look at the PR, thanks!

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

No branches or pull requests

2 participants