-
Notifications
You must be signed in to change notification settings - Fork 132
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
improve require/import/export support in unminify #30
Comments
Yeah it needs to get reworked at some point... the tricky part is detecting if a module is esm or not if it doesn't use The default export is commonly called But it's still accessed like any other named export: https://github.com/swc-project/swc/blob/3550d0be26ebab7bcb7942e1a12a7d5fc3aedb17/crates/swc_ecma_minifier/tests/fixture/projects/next/archive-2/pages/hmr/.counter-c6a48518c4a56e4d9ce5/input.js#L230 |
Forgive my naivety, but would it matter either way? Could it just be a user override setting of like 'always produce ESM', even if the original might not have been? Or would it introduce issues into the code that would make it not work/etc? I guess based on this note (Ref) it seems there may be incompatibilities if this was done. |
And because you can't import esm from commonjs but that's impossible to validate with multiple chunks or other code we don't know about Here's a wip version that converts all top level requires and this export variation: https://deploy-preview-31--webcrack.netlify.app/ import { jsx } from /*webcrack:missing*/"./35250.js";
export function Z(e) {
var t;
var n = e.url;
var a = e.size;
var i = a === undefined ? 16 : a;
var s = e.className;
try {
t = new URL(n);
} catch (e) {
console.error(e);
return null;
}
return <img src={`https://icons.duckduckgo.com/ip3/${t.hostname}.ico`} alt="Favicon" width={i} height={i} className={s} />;
} |
@j4k0xb That makes sense for code we don't know about, but why is it impossible to validate for multiple chunks? Is that just because webcrack only operates on a single chunk at a time currently? (Ref)
Nice! That's looking so much better/more useful already! Using that same example code (Ref), with the new preview version (Ref), looking at DetailsTop of file imports: // 63390.js, lines 1-24
var r;
import { _ as _2 } from /*webcrack:missing*/"./39324.js";
import { _ as _3 } from /*webcrack:missing*/"./70216.js";
import { _ as _4 } from /*webcrack:missing*/"./4337.js";
import { jsxs, Fragment, jsx } from /*webcrack:missing*/"./35250.js";
import { Z as _Z } from /*webcrack:missing*/"./19841.js";
import { useRouter } from /*webcrack:missing*/"./68555.js";
import { useId, useState, useRef, useEffect, useCallback, forwardRef, Children, useMemo, memo } from /*webcrack:missing*/"./70079.js";
import { Z as _Z2 } from /*webcrack:missing*/"./34303.js";
import { Dd } from "./75179.js";
import { hz } from /*webcrack:missing*/"./64135.js";
import { FZ } from /*webcrack:missing*/"./88038.js";
import { tQ } from "./75527.js";
import { Qd } from "./36716.js";
import { wp } from /*webcrack:missing*/"./59710.js";
import { RR } from "./56244.js";
import { z as _z } from /*webcrack:missing*/"./66958.js";
import { Z as _Z3 } from "./30931.js";
import { Z as _Z4 } from "./87105.js";
import { _ as _5 } from /*webcrack:missing*/"./22830.js";
import { ZP as _ZP } from /*webcrack:missing*/"./2827.js";
import { WS } from /*webcrack:missing*/"./82841.js";
import { s6 } from /*webcrack:missing*/"./36218.js";
import { Os, uU } from "./69403.js"; Part way down, non-hoisted: // 63390.js, lines 108-115
import { _ } from /*webcrack:missing*/"./21722.js";
import { Jh } from /*webcrack:missing*/"./39889.js";
import { h } from /*webcrack:missing*/"./10642.js";
import { get } from /*webcrack:missing*/"./47635.js";
import { kP } from /*webcrack:missing*/"./32787.js";
import { ZP as _ZP2 } from /*webcrack:missing*/"./24274.js";
import { Z as _Z5 } from "./75515.js";
import { Z as _Z6 } from "./180.js"; With From memory, this web app is bundled with Looking at the same file/output in DetailsTop of FileSource (unpacked) // module-63390.js, lines 1-24
var r;
var a = require(39324);
var i = require(70216);
var s = require(4337);
var o = require(35250);
var l = require(19841);
var u = require(68555);
var d = require(70079);
var c = require(34303);
var f = require(75179);
var h = require(64135);
var g = require(88038);
var m = require(75527);
var p = require(36716);
var v = require(59710);
var x = require(56244);
var b = require(66958);
var y = require(30931);
var w = require(87105);
var j = require(22830);
var _ = require(2827);
var C = require(82841);
var M = require(36218);
var k = require(69403); Transformed (unminified) Note that // module-63390.js, lines 1-49
import f from "module-75179.js";
import m from "module-75527.js";
import p, { Qd } from "module-36716.js";
import x, { RR } from "module-56244.js";
import y from "module-30931.js";
import w from "module-87105.js";
import k from "module-69403.js";
import R from "module-75515.js";
import U from "module-180.js";
import Y, { dQ } from "module-77442.js";
import { v as v$0 } from "module-31721.js";
import ec, { Y8, m0 } from "module-5046.js";
import { Iy } from "module-25094.js";
let r;
const { _: _$5 } = require(39324);
const { _: _$4 } = require(70216);
const { _: _$0 } = require(4337);
const o = require(35250);
const { jsxs, jsx } = o;
const { Z: Z$0 } = require(19841);
const { useRouter } = require(68555);
const d = require(70079);
const { useId, useState, useRef, useEffect, useCallback, forwardRef, useMemo } =
d;
const c = require(34303);
const { hz } = require(64135);
const g = require(88038);
const v = require(59710);
const b = require(66958);
const { _: _$1 } = require(22830);
const _ = require(2827);
const { WS } = require(82841);
const M = require(36218); Part way downSource (unpacked) // module-63390.js, lines 138-146
var S = c.Z.div(N());
var I = require(21722);
var F = require(39889);
var E = require(10642);
var D = require(47635);
var L = require(32787);
var A = require(24274);
var R = require(75515);
var U = require(180); Transformed (unminified) // module-63390.js, lines 170-182
var S = c.Z.div(N());
const { _: _$3 } = require(21722);
const { Jh } = require(39889);
const { h: h$0 } = require(10642);
const D = require(47635);
const { kP } = require(32787);
const A = require(24274); Note that in the 'unpacked' for this section, there are // module-63390.js, lines 8-9
import R from "module-75515.js";
import U from "module-180.js"; |
Yes, or rather a lot of work to do it "properly". Requires operating on all chunks and building an import/require graph like in pionxzh/wakaru#73.
I think it was a mistake to convert all requires, what wakaru does with not touching the 'part way down' ones is more correct.
I doubt imports are ever moved down so it's probably a module with only require or both import/require (which webpack can bundle). |
@j4k0xb nods, yeah, makes sense. If you haven't seen them yet, some of these tools/libs I found the other day for visualising dependency graphs could potentially be useful for the module graph concept (Ref) I wonder if even 'not proper' support would be useful to add, and how hard that would be. I've found
I haven't deeply looked into this, and not for ages, but at one stage I remember having a thought that the chunks specified the other chunks they depended on somewhere (as well as the individual module imports within it) (Ref) In the code I was most exploring, theres the Looking at a fairly small/basic chunk, it seems like it doesn't have anywhere that specifies dependencies on other chunks (Ref) But then looking at a far larger chunk file ( function (U) {
var B = function (B) {
return U((U.s = B));
};
U.O(0, [774, 179], function () {
return B(18992), B(9869), B(76281);
}),
(_N_E = U.O());
},
@j4k0xb nods makes sense. I'm not sure of the heuristics it uses to decide if they can be converted to
@j4k0xb Yeah, the 'import/require' part was what my original theory was. |
The current behavior of wakaru is to convert all top-level I'm not sure if we can tell if a |
@pionxzh I haven't looked into this too deeply, but in cases where there is something similar to this, it should be safe to assume it's truly an ESM module (and so would be safe to convert to Object.defineProperty(exports, "__esModule", { value: true }); Like this sort of thing:
@pionxzh Another case that could potentially be checked is whether the imported module is 'pure' or not. Eg. If it runs code in the main body of the file / an IIFE then it probably(?) wouldn't necessarily be safe to convert it to Not sure off the top of my head, but there might also be some other 'heuristics' of things that only appear/only are supported in ESM modules (or vice versa), that could be looked for to determine if it's safe to convert |
afaik some more heuristics:
I looked at many of your samples and sometimes esm appears to be commonjs because of variable inlining or other build tools. var m = __webpack_require__(57311); // import ... from '57311', how webpack normally creates it
__webpack_require__(44675).env.INTERNAL_API_URL; // was probably `var n = __webpack_require__(44675); n.env.INTERNAL_API_URL` or https://github.com/0xdevalias/chatgpt-source-watch/blob/main/orig/_next/static/chunks/293-defd068c38bd0c8d.js module 12285, temporary variables from typescript enums but it should detect and hoist imports: __webpack_require__.d(exports, {
R: function () {
return V;
},
});
var r;
var o;
var i;
var a;
var u = __webpack_require__(70079);
var c = __webpack_require__(62530);
var s = __webpack_require__(19430);
var l = __webpack_require__(9335);
var f = __webpack_require__(41800);
function p(t, e) {
let [n, r] = (0, u.useState)(t);
let o = (0, f.E)(t);
(0, l.e)(() => r(o.current), [o, r, ...e]);
return n;
}
var v = __webpack_require__(84325);
// ...
(r = P || {})[r.Open = 0] = "Open";
r[r.Closed = 1] = "Closed";
var P = r;
did some more tests and apparently webpack is able to bundle it, even mixed commonjs/esm in a single module |
#50 / https://deploy-preview-50--webcrack.netlify.app/ contains a rewrite of the whole unpacking/esm logic (very experimental) |
Was testing the new v2.11.0 web IDE update today and found some areas that unminify could be further improved.
This issue is about the require/import/export syntax after unminification.
You can get the minimised code that I am testing against here (Ref, that repo also has lots of other example code if you want to test further)
Webcrack
Loading the above minimised code in the webcrack web IDE (Ref) with the following config:
In the unminified output, you can see an example of the current require/import/export in
180.js
:(Note: I also used this same file as my example for the JSX unminify in #10 (comment), but the specifics in this issue are different, despite it using the same source)
(Sidenote: I love the
/*webcrack:missing*/
annotation here!)180.js
Wakaru
Contrasting this against the unminified output from
wakaru
's web IDE (Ref) (which separates the 'unpack modules' and 'unminify' steps)Unpacked
module-180.js
source:Transformed (unminified)
module-180.js
source:The text was updated successfully, but these errors were encountered: