aurelia / webpack-plugin

A plugin for webpack that enables bundling Aurelia applications.
MIT License
90 stars 36 forks source link

refactor: change import for webpack v5 | not ready #169

Closed deleonio closed 3 years ago

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

deleonio commented 3 years ago

image

jods4 commented 3 years ago

@martinoppitz Thank you for having a go at this, much appreciated!

I see this is not ready but I'll make two initial comments:

  1. Please don't commit dist. It is built and committed by the release process.

  2. It's great that you tried to maintain compat with v4 as well. If that becomes too hairy, just drop it. We'll make a plugin v5 release compatible with webpack 5, and webpack 4 users can stay on the plugin v4.

deleonio commented 3 years ago

Hello @jods4,

in my first commit I focused on v5 upgrade. Now I reset my v4 changes and pushed it again.

Now we must look at the iterator problem.

deleonio commented 3 years ago

Hi @jods4 and @EisenbergEffect,

in the plugin an internal webpack.d.ts file is used instead of the one from webpack itself.

there seem to be some changes here that i can't fix on my own.

jods4 commented 3 years ago

@martinoppitz I wrote that .d.ts file to augment the public one. I haven't checked recently but at the time this plugin was first written, only common "webpack-user" APIs were documented in its typescript definitions. Practically none of the "internal" API geared at "plugin-writters" were typed. So I augmented the public definitions with the stuff I needed to use to create this plugin.

These definitions are almost guaranteed to not be in sync with the Webpack 5. You can remove anything that's now in the official webpack.d.ts, doesn't exist anymore, or isn't used by the plugin -- and figure out the correct type defs for whatever you use that's not in the official one.

deleonio commented 3 years ago

image

https://github.com/martinoppitz/poc-flexible-web-application-architecture/actions/runs/397574537

deleonio commented 3 years ago

Hello @jods4.

unfortunately i don't have time to get the webpack plugin running.

We must upgrade webpack to v5. Because a lot of plugins only support webpack v5 in the future.

Do you already have a plan?

ckotzbauer commented 3 years ago

I started investigating here and used the changes from this PR as base. I changed the code of PreserveExportsPlugin.ts and PreserveModuleNamePlugin.ts to fix the "reasons not iterable" problem (I don't know if I really fixed it, but the error is gone). But now I get the next error-message from webpack:

Module not found: Error: Can't resolve './src' in '/mnt/dev/aurelia-client'
resolve './src' in '/mnt/dev/aurelia-client'
  using description file: /mnt/dev/aurelia-client/package.json (relative path: .)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: /mnt/dev/aurelia-client/package.json (relative path: ./src)
      no extension
        Field 'browser' doesn't contain a valid alias configuration
        /mnt/dev/aurelia-client/src doesn't exist
      .ts
        Field 'browser' doesn't contain a valid alias configuration
        /mnt/dev/aurelia-client/src.ts doesn't exist
      .js
        Field 'browser' doesn't contain a valid alias configuration
        /mnt/dev/aurelia-client/src.js doesn't exist
      as directory
        /mnt/dev/aurelia-client/src doesn't exist

webpack 5.21.1 compiled with 2 errors in 1030 ms

There is no src folder, as the aurelia app is located in a app folder. But I don't see any parameter of this plugin or webpack itself to change this. Does anybody know? :thinking:

jods4 commented 3 years ago

@ckotzbauer Hard to say from this error stack but have you tried changing the default options? viewFor assumes a src folder: https://github.com/aurelia/webpack-plugin/wiki/AureliaPlugin-options#viewfor-and-viewextensions

ckotzbauer commented 3 years ago

I explicitly set viewsFor to app/**/*.{ts,js}. There are no other options of this plugin with a src default value:

this.options = Object.assign({
      includeAll: <false>false,            
      aureliaConfig: ["standard", "developmentLogging"],
      dist: "native-modules",
      features: { },
      moduleMethods: [],
      noHtmlLoader: false,
      // Undocumented safety switch
      noInlineView: false,
      noModulePathResolve: false,
      noWebpackLoader: false,
      // Ideally we would like _not_ to process conventions in node_modules,
      // because they should be using @useView and not rely in convention in 
      // the first place. Unfortunately at this point many libs do use conventions
      // so it's just more helpful for users to process them.
      // As unlikely as it may seem, a common offender here is tslib, which has
      // matching (yet unrelated) html files in its distribution. So I am making 
      // a quick exception for that.
      viewsFor: "**/!(tslib)*.{ts,js}",
      viewsExtensions: ".html",
    },
    options);
gowrav-scienaptic commented 3 years ago

@ckotzbauer, Can you please post the changes you have done to PreserveExportsPlugin.ts and PreserveModuleNamePlugin.ts

ckotzbauer commented 3 years ago

Of course @gowrav-scienaptic :+1:

PreserveExportsPlugin.ts

        for (let module of modules) {
+          for (let dep of module.dependencies) {
-          for (let reason of module.reasons) {
-            let dep = reason.dependency;
            let imports = dep[dependencyImports];
            if (!imports) continue;            
            let set = getModuleExports(module);
            for (let e of imports)
              set.add(e);
          }
        }

PreserveModuleNamePlugin.ts

function getPreservedModules(modules: Webpack.Module[]) {      
  return new Set(
    modules.filter(m => {
      // Some modules might have [preserveModuleName] already set, see ConventionDependenciesPlugin.
      let value = m[preserveModuleName];      
+      for (let d of m.dependencies) {
+        if (!d || !d[preserveModuleName]) continue;
-      for (let r of m.reasons) {
-        if (!r.dependency || !r.dependency[preserveModuleName]) continue;
        value = true;
+        let req = removeLoaders((d as ModuleDependency).request);
-        let req = removeLoaders((r.dependency as ModuleDependency).request);
        // We try to find an absolute string and set that as the module [preserveModuleName], as it's the best id.
        if (req && !req.startsWith(".")) {
          m[preserveModuleName] = req;
          return true;
        }
      }
      return !!value;
    })
  );
}
graycrow commented 3 years ago

... But now I get the next error-message from webpack:

Module not found: Error: Can't resolve './src' in '/mnt/dev/aurelia-client'
resolve './src' in '/mnt/dev/aurelia-client'
...

There is no src folder, as the aurelia app is located in a app folder. But I don't see any parameter of this plugin or webpack itself to change this. Does anybody know? thinking

I have exactly the same error, but I have the default ./src directory.

gowrav-scienaptic commented 3 years ago

module.reasons can't be replaced with module.dependencies as shown above. Tried it on Aurelia skeleton app and it doesn't work. Unable to find the equivalent of module.reasons in webpack 5.

bigopon commented 3 years ago

Should reasons of a module be understood as issuers or dependencies?

Devvox93 commented 3 years ago

I have done some searching and (possibly) found a solution/workaround for the removal of module.reasons in this comment.

I was checking module.reasons but it was removed in webpack 5. My workaround was calling compilation.getStats() which still contains the reasons...

Update 1: I have fooled around a bit with the source, but I'm not educated enough in how Webpack works exactly to work this out. Perhaps @ScriptedAlchemy can shed some light on how to get the reasons (or similar information) in Webpack 5. The release notes simply state "Module.reasons removed", without any info on migration or alternatives.

Update 2: In #166 bigopon is working on supporting webpack 5 the proper way. I suspect this PR will become unnecessary.

bigopon commented 3 years ago

Here is my current draft for the needed changes: #170. The work needed for all the plugins is listed in 3 simple parts for now: typings & code & comments. Overtime, more details will be filled in. Will update my progress there as I continue the refactoring.

deleonio commented 3 years ago

Its done ... https://github.com/aurelia/webpack-plugin/pull/170