aurelia / webpack-plugin

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

fix: broken webpack mangling of harmony imports in production mode #223

Closed vilnytskyi closed 1 year ago

vilnytskyi commented 1 year ago

There is an issue when bundling code with imported aurelia dependency via harmony import. The code like:

import { ConsoleAppender } from 'aurelia-logging-console' console.log(ConsoleAppender) bundles into something like:

var a=r("aurelia-logging-console") console.log(a.A) while aurelia-logging-console isn't mangled itself to match the export names, so i.I is undefined.

This fix seem to work for all such cases but I am not sure if the original approach was written in such way for some specific cases which I might not be aware of so better to be investigated by core team.There is an issue when bundling code with imported aurelia dependency via harmony import. The code like:

import { ConsoleAppender } from 'aurelia-logging-console' console.log(ConsoleAppender) bundles into something like:

var a=r("aurelia-logging-console") console.log(a.A) while aurelia-logging-console isn't mangled itself to match the export names, so i.I is undefined.

This fix seem to work for all such cases but I am not sure if the original approach was written in such way for some specific cases which I might not be aware of so better to be investigated by core team.

Credits to @alexander-akait for pointing this out!

bigopon commented 1 year ago

On any module that is referenced via PLATFORM.moduleName, which under the hood AureliaDependency, there' 2 things that need to happen:

The existing code is to make sure the 2nd part work. So will changing it to EXPORTS_OBJECT_REFERENCED make it work the same way?

@alexander-akait is there a better way to express the above requirement to webpack? At the moment, with AureliaDependency, which extends ModuleDependency, it results in non harmony import (legacy/requirejs), which causes problems.

An example of the issues is like in my analysis here https://github.com/aurelia/webpack-plugin/issues/206#issuecomment-1584337724

image

In the above code block, line 5 results in a reference to @casl/ability/dist/umd/index.js instead of the module one at @casl/ability/dist/es6m/index.js

alexander-akait commented 1 year ago

@bigopon

On any module that is referenced via PLATFORM.moduleName, which under the hood AureliaDependency, there' 2 things that need to happen:

the module must not be concatenated, and the module name must be preserved all the exports must be preserved as is (no mangled)

In such case this PR fix it.

Also in future we can implement more optimizations for commonjs, we don't implement all of them, because many of them are very rare.

But if you want to keep all exports and don't mangle it you should return nothing, so EXPORTS_OBJECT_REFERENCED is [[]], so you say webpack (siplified) - yes, we have export, but we don't want to change an export

bigopon commented 1 year ago

This has been released as 5.0.6 Thanks @vilnytskyi @alexander-akait