aurelia / loader-webpack

An implementation of Aurelia's loader interface to enable webpack.
MIT License
26 stars 10 forks source link

WebpackPlugin.loadText() returns object instead of string with webpack/aurelia-webpack-plugin 5 #59

Closed josundt closed 3 years ago

josundt commented 3 years ago

I have spent a couple of days getting aurelia-webpack-plugin@5.0.0 and webpack 5 to work with our Aurelia 1 based products.

I found a few issues with that I will submit fixes to in separate PR(s) to aurelia-webpack-plugin repository, but I also got an error that needs to be fixed in the aurelia-loader-webpack package.

The problem I found is in the WebpackLoader.loadText method. After upgrading to webpack 5 and aurelia-webpack-plugin 5, this method returned an object instead of a string when loading HTML templates.

The object that was returned from the method instead of the expected string looked like this:

{ 
  get default(): "<template>....", // The template
  Symbol(Symbol.toStringTag): "Module"
  __esModule: true
}

To fix this issue so that it will return a string for webpack/aurelia-webpack-plugin 5 without breaking the behavior when used with webpack/aurelia-webpack-plugin 4, I simply modified the return statement on the last line of the function, since the value of the defaultExport variable is the string that should be returned:

export class WebpackLoader extends Loader {
  //...
  async loadText(url: string) {
    const result = await this.loadModule(url, false);
    // css-loader could use esModule:true
    const defaultExport = result && result.__esModule ? result.default : result;
    if (defaultExport instanceof Array && defaultExport[0] instanceof Array && defaultExport.hasOwnProperty('toString')) {
      // we're dealing with a file loaded using the css-loader:
      return defaultExport.toString();
    }
    // CHANGE START
    // before: 
    // return result;
    // after:
    return typeof result === "string" ? result : defaultExport;
    // CHANGE END
  }
  //...
}

PS! I also needed to make one more small change to the existing code for the typescript compilation to succeed. There was a name collision between the exported type LoaderPlugin and the imported type LoaderPlugin from aurelia-loader. To resolve the conflict I gave the imported type an alias.

@EisenbergEffect I hope this approval of the PR and release of a new version will be prioritized ASAP (even before looking at the pending dependabot PRs), as I am confident that this plus the other few fixes that I plan to submit to the aurelia-webpack-plugin repository will improve the overall robustness of Aurelia 1 with webpack 5. The webpack configuration used in our products is based on the aurelia cli generated webpack configuration, but over time got some more advanced configuration options. The testing I have done with our products probably revealed a few issues that may not have been covered by earlier tests.

Background: The company I work for has a product suite with multiple TypeScript/Webpack based Aurelia 1 apps that share a common Aurelia 1 UI library and many other components like f.ex. a common webpack builder library.

bigopon commented 3 years ago

Thanks @josundt I think the fix looks safe for a release.

bigopon commented 3 years ago

I'll coordinate a release with @EisenbergEffect

josundt commented 3 years ago

@bigopon I have now found that there also seems to be another runtime issue with loading html templates when webpack mode=production. I am currently investigating it and hope to find out some more.

I am no expert on webpack plugins, so I created a repository with a simple app (created with aurelia-cli, and then migrated to webpack 5) that demonstrates how this problem only exists when mode=production (--env production).

To reproduce: The app works perfectly with the standard npm start, but when running the app with the added script npm run start:prod (which does the same thing as nmp start - only with --env production), console errors appear in the browser.

https://github.com/josundt/aurelia1-wp5-bug-repro

I'll test some more and let you know if I am able to create a fix, if not maybe someone else can use the repo above to figure it out.