Skip to content

[Vue] Add polyfills#19176

Closed
michaeltintiuc wants to merge 1 commit into
ionic-team:masterfrom
ModusCreateOrg:add-polyfills
Closed

[Vue] Add polyfills#19176
michaeltintiuc wants to merge 1 commit into
ionic-team:masterfrom
ModusCreateOrg:add-polyfills

Conversation

@michaeltintiuc

Copy link
Copy Markdown
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Without calling the applyPolyfills function MS Edge fails with the following error:
Unhandled promise rejection TypeError: Unable to get property 'get' of undefined or null reference

What is the new behavior?

MS Edge no longer fails

Does this introduce a breaking change?

  • Yes
  • No

@ionitron-bot ionitron-bot Bot added the package: vue @ionic/vue package label Aug 25, 2019
@michaeltintiuc michaeltintiuc changed the title Add polyfills [Vue] Add polyfills Aug 25, 2019
@zwacky

zwacky commented Aug 26, 2019

Copy link
Copy Markdown
Contributor

I wonder what applyPolyfills() actually does, because in the source it's simply an empty Promise.resolve.

Is that portion added through the build process?

@manucorporat

Copy link
Copy Markdown
Contributor

@zwacky that is only for CDN use case, ie, loading ionic in stackblitz for example.

@zwacky

zwacky commented Aug 26, 2019

Copy link
Copy Markdown
Contributor

I can't fully understand why this little code, which is merely a return Promise.resolve() breaks MS Edge user from rendering at all 🤔
But this change fixes the problem (!)

@michaeltintiuc

Copy link
Copy Markdown
Contributor Author

Based on my debug results, this is the function that is actually being called:
Loaded from @ionic/core/loader/index.mjs and the contents being in @ionic/core/dist/esm/polyfills/index.js

export function applyPolyfills() {
  var win = window;

  var promises = [];

  if (!win.customElements || (win.Element && (!win.Element.prototype.closest || !win.Element.prototype.matches || !win.Element.prototype.remove))) {
    promises.push(import('./dom.js'));
  }

  function checkIfURLIsSupported() {
    try {
      var u = new URL('b', 'http://a');
      u.pathname = 'c%20d';
      return (u.href === 'http://a/c%20d') && u.searchParams;
    } catch(e) {
      return false;
    }
  }

  if (
    'function' !== typeof Object.assign || !Object.entries ||
    !Array.prototype.find || !Array.prototype.includes ||
    !String.prototype.startsWith || !String.prototype.endsWith ||
    (win.NodeList && !win.NodeList.prototype.forEach) ||
    !win.fetch ||
    !checkIfURLIsSupported() ||
    typeof WeakMap == 'undefined'
  ) {
    promises.push(import('./core-js.js'));
  }
  if (!(win.CSS && win.CSS.supports && win.CSS.supports('color', 'var(--c)'))) {
    promises.push(import('./css-shim.js'));
  }
  return Promise.all(promises);
}

@michaeltintiuc michaeltintiuc deleted the add-polyfills branch August 27, 2019 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants