Skip to content
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

Ensure compatibility with Terser pure_getters optimization #2770

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

earshinov
Copy link

Hello!

This change makes Quill source compatible with Terser pure_getters optimization.

This change is needed for Angular projects, because:

  1. Angular CLI (the most popular tool for building Angular projects) internally uses Webpack with terser-webpack-plugin and unconditionally applies pure_getters optimization. You can see all of it in Angular CLI sources: webpack-configs/common.ts.

    At some point Angular CLI team disabled pure_getters optimization (see Production build optimization breaks code angular/angular-cli#11439, fix(@angular-devkit/build-angular): remove pure_getters angular/angular-cli#14187), but then they returned it back for their reasons (Revert "fix(@angular-devkit/build-angular): remove pure_getters" angular/angular-cli#14287).

  2. Angular CLI not only compiles project code, but also recompiles the code of external libraries located in node_modules, including quill.min.js if it happens to be there

Here is the problematic code from Quill:

function contains(parent, descendant) {
  try {
    // Firefox inserts inaccessible nodes around video elements
    descendant.parentNode; // eslint-disable-line no-unused-expressions
  } catch (e) {
    return false;
  }
  return parent.contains(descendant);
}

With pure_getters optimization, Terser duly assumes that descendant.parentNode statement is not doing anything and prunes the whole try..catch block. You can easily reproduce the problem locally:

$ terser -c pure_getters <<EOF
export function contains(parent, descendant) {
  try {
    // Firefox inserts inaccessible nodes around video elements
    descendant.parentNode; // eslint-disable-line no-unused-expressions
  } catch (e) {
    return false;
  }
  return parent.contains(descendant);
}
EOF
export function contains(parent,descendant){return parent.contains(descendant)}

Of course, the resulting code will throw the Error described in #644 because the guards aren't there.

@luin luin added this to the 2.0 milestone Dec 15, 2022
@luin luin self-requested a review December 15, 2022 01:31
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

Successfully merging this pull request may close these issues.

None yet

2 participants