Skip to content

Improve tree shakability #608

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

Closed
fisker opened this issue May 12, 2025 · 5 comments
Closed

Improve tree shakability #608

fisker opened this issue May 12, 2025 · 5 comments

Comments

@fisker
Copy link
Contributor

fisker commented May 12, 2025

echo export{convertChangesToDMP}from"diff" | npx esbuild --bundle --format=esm

Expect output to only include convertChangesToDMP, instead of 800+ LOC

@fisker
Copy link
Contributor Author

fisker commented May 15, 2025

Sorry I didn't notice that files in libesm/ are exported, I should import from the file directly instead of the index file.

Great work! Thanks!

@fisker fisker closed this as not planned Won't fix, can't repro, duplicate, stale May 15, 2025
@ExplodingCabbage
Copy link
Collaborator

ExplodingCabbage commented May 16, 2025

Yep, I put that feature in specifically to avoid breaking the imports in Prettier that I recalled imported from lib/ (though apparently that got changed in Pretter in the meantime and I hadn't noticed).

I am curious why esbuild can't manage to notice that those 800 lines of extra code are unused and purge them from the output, though! It's not obvious to me that importing directly from diff should give a different result at all.

@fisker
Copy link
Contributor Author

fisker commented May 16, 2025

I put that feature in specifically to avoid breaking the imports in Prettier that I recalled imported from lib/ (though

Thanks! Yes, we imported from lib/ directly before, but changed for some reason I can't remember. prettier/prettier#16397

I am curious why esbuild can't manage to notice that those 800 lines of extra code are unused and purge them from the output

I guess that's because lines like this

export const sentenceDiff = new SentenceDiff();

were not marked as "pure", can also caused by tansformed classes, I didn't dig, just my guess.

@fisker
Copy link
Contributor Author

fisker commented May 16, 2025

I wonder if put sideEffects: false in package.json will make the extra code go away.

@fisker
Copy link
Contributor Author

fisker commented May 16, 2025

As I tested, sideEffects: false doesn't work.

My guess is correct, if I do

Image

to all the files in libesm/

, I got

>echo export{convertChangesToDMP}from"diff" | npx esbuild --bundle --format=esm
// node_modules/diff/libesm/diff/word.js
var extendedWordChars = "a-zA-Z0-9_\\u{C0}-\\u{FF}\\u{D8}-\\u{F6}\\u{F8}-\\u{2C6}\\u{2C8}-\\u{2D7}\\u{2DE}-\\u{2FF}\\u{1E00}-\\u{1EFF}";
var tokenizeIncludingWhitespace = /* @__PURE__ */ new RegExp("[".concat(extendedWordChars, "]+|\\s+|[^").concat(extendedWordChars, "]"), "ug");

// node_modules/diff/libesm/convert/dmp.js
function convertChangesToDMP(changes) {
  var ret = [];
  var change, operation;
  for (var i = 0; i < changes.length; i++) {
    change = changes[i];
    if (change.added) {
      operation = 1;
    } else if (change.removed) {
      operation = -1;
    } else {
      operation = 0;
    }
    ret.push([operation, change.value]);
  }
  return ret;
}
export {
  convertChangesToDMP
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants