Skip to content

Conversation

@j4k0xb
Copy link
Owner

@j4k0xb j4k0xb commented Feb 17, 2024

closes #62

@netlify
Copy link

netlify bot commented Feb 17, 2024

Deploy Preview for webcrack ready!

Name Link
🔨 Latest commit 271f21c
🔍 Latest deploy log https://app.netlify.com/sites/webcrack/deploys/65d3de47ff1280000792e0af
😎 Deploy Preview https://deploy-preview-63--webcrack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@j4k0xb j4k0xb force-pushed the rename-destructuring branch from c8f4452 to 16f575f Compare February 17, 2024 16:55
@milahu
Copy link
Contributor

milahu commented Feb 18, 2024

testing this with a 3MB input file

rename should also affect imports, but this is also missing in wakaru

import {
  render as ak,
  html as aq,
  Component as az,
} from "/libs/preact/preact.js";

console.log(ak, aq, az);

some rebindings are preserved
because the identifier has already been declared, regardless of scope

i guess an optimal solution would require dynamic analysis
for example to ensure that Math == Math
so that Math.sqrt == Math.sqrt
so the first let { sqrt } = Math; can be reused
and the following let { sqrt: _sqrtN } = Math; can be removed

let { sqrt } = Math;
let { sqrt: _sqrt } = Math;
let { sqrt: _sqrt2 } = Math;
let { sqrt: _sqrt3 } = Math;

works for y, fails on x, because x is declared globally

const x = "...";

let { x: _x2, y } = aD;
let { x: _x4, y } = DOMPoint.fromPoint(aP).matrixTransform(aN);
let { x: _x5, y } = DOMPoint.fromPoint(aV).matrixTransform(aT);
let { x: _x6, y } = Ha(...);
let { x: _x7, y } = Ha(...);

here it also fails on y, because y is declared locally

let { x: _x8, y } = aF.matrixTransform(aD);
let { x: _x9, y: _y } = aG.matrixTransform(aD);

@j4k0xb
Copy link
Owner Author

j4k0xb commented Feb 19, 2024

now also affects imports

i guess an optimal solution would require dynamic analysis
for example to ensure that Math == Math

can maybe be done later/separately because its not related to the renaming and potentially unsafe
I guess this was created by a bundler with scope hoisting (all modules are concatenated) like rollup/esbuild/swc

works for y, fails on x, because x is declared globally

hmm could be improved if it wont affect the performance too much
it currently doesn't use x to avoid checking for shadowing:

const x = "...";
function f() {
  let { x: _x2, y } = aD;
  console.log(x);
}

here it also fails on y, because y is declared locally

expected

@milahu
Copy link
Contributor

milahu commented Feb 20, 2024

rename should also affect imports

similar situation with custom elements

webcrack should infer the class name from the element name

class aa extends HTMLElement {
  static #V = html`<div>foo</div>`;
  static #j = css`:host { display: block; }`;
  #Y;
  constructor() {
    super();
    this.#Y = this.attachShadow({  mode: "open" });
    this.#Y.adoptedStyleSheets = [aa.#j];
    this.#Y.append(document.importNode(aa.#V.content, true));
  }
}
customElements.define("x-foo", aa);
class XFoo extends HTMLElement {
  // ...
}
customElements.define("x-foo", XFoo);

maybe also add names to anonymous classes

customElements.define(
  "x-foo",
  class extends HTMLElement { /* ... */ }
);

@0xdevalias
Copy link
Contributor

i guess an optimal solution would require dynamic analysis
for example to ensure that Math == Math

can maybe be done later/separately because its not related to the renaming and potentially unsafe

I haven't looked into how this is implemented currently, nor an example case of this code.. but I would imagine that we should be able to at least confirm this partially based on the scopes and whether anything is modifying / writing to that Math symbol, right?

That wouldn't really tell us if Math is really 'the Math module', but it should presumably be able to tell us that in a large number of cases that these are all equivalent:

let { sqrt } = Math;
let { sqrt: _sqrt } = Math;
let { sqrt: _sqrt2 } = Math;
let { sqrt: _sqrt3 } = Math;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add smart-rename rule from wakaru

4 participants