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

Minify compile time constants during build #142

Closed
wants to merge 1 commit into from

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Jul 3, 2018

Previously SEND_RESPONSE_DEPRECATION_WARNING would end up as:

  const SEND_RESPONSE_DEPRECATION_WARNING = `
      Returning a Promise is the preferred way to send a reply from an
      onMessage/onMessageExternal listener, as the sendResponse will be
      removed from the specs (See
      https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onMessage)
    `.replace(/\s+/g, " ").trim();

in both the built and minified distribution files, which adds unnecessary start-up overhead where the /\s+/g regular expression has to be compiled and executed, followed by trimming the string.

This instead performs such a process during the build, which provides a few milliseconds of start-up time savings per extension which uses this polyfill, which adds up for a lot of extensions, especially ones with an event page, where the polyfill has to be loaded and executed every time the extension handles an event.

@ExE-Boss ExE-Boss mentioned this pull request Jul 3, 2018
@rpl
Copy link
Member

rpl commented Jul 17, 2018

I would prefer to don't do this, it looks like a bit of a micro-optimization but, if we really want to spare those milliseconds (e.g. if we have some kind of proof that for some extension those milliseconds are really going to sum up and become an actual performance issue), I would prefer to just rewrite that const to become a single-line string right into the polyfill sources (without applying these changes at build time).

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