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

Polyfill missing std lib fns for module browsers #17083

Merged
merged 6 commits into from
Sep 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/next-polyfill-module/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "@next/polyfill-module",
"version": "9.5.4-canary.16",
"description": "A standard library polyfill for ES Modules supporting browsers (Edge 16+, Firefox 60+, Chrome 61+, Safari 10.1+)",
"main": "dist/polyfill-module.js",
"license": "MIT",
"repository": {
"url": "vercel/next.js",
"directory": "packages/next-polyfill-module"
},
"scripts": {
"prepublish": "microbundle src/index.js -f iife --no-sourcemap --external none",
"build": "microbundle watch src/index.js -f iife --no-sourcemap --external none"
},
"devDependencies": {
"microbundle": "0.11.0"
}
}
94 changes: 94 additions & 0 deletions packages/next-polyfill-module/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/* eslint-disable no-extend-native */

// Contains polyfills for methods missing after browser version(s):
// Edge 16, Firefox 60, Chrome 61, Safari 10.1

/**
* Available in:
* Edge: never
* Firefox: 61
* Chrome: 66
* Safari: 12
*
* https://caniuse.com/mdn-javascript_builtins_string_trimstart
* https://caniuse.com/mdn-javascript_builtins_string_trimend
*/
if (!('trimStart' in String.prototype)) {
String.prototype.trimStart = String.prototype.trimLeft
}
if (!('trimEnd' in String.prototype)) {
String.prototype.trimEnd = String.prototype.trimRight
}

/**
* Available in:
* Edge: never
* Firefox: 63
* Chrome: 70
* Safari: 12.1
*
* https://caniuse.com/mdn-javascript_builtins_symbol_description
*/
if (!('description' in Symbol.prototype)) {
Object.defineProperty(Symbol.prototype, 'description', {
get: function get() {
return /\((.+)\)/.exec(this)[1]
},
})
}
Comment on lines +32 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijjk This polyfill doesn't work properly on Edge 44 (#18213) and Safari 11 (#17825). We can't upgrade to 9.5.4 to fix the security alert and also 10.0 today... Does Next.js need this polyfill internally? If not, could we remove it for now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you send a PR to fix it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can open a PR to fix the issue on Edge 44 with a small change

Object.defineProperty(Symbol.prototype, 'description', {
  get: function get() {
    const result = /\((.+)\)/.exec(this)[1];
    return result ? result[1] : undefined;
  },
})

However, this still doesn't handle the other issue with Safari 11 (#17825). Core-js es.symbol.description polyfill seems to be more complete (handling all browser) but I'm not sure if we want to bring all that code into Next.js. I would suggest that we remove this problematic polyfill from Next.js. Whoever needs it can polyfill it manually.

@Timer WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core-js does not polyfill symbol.description, it polyfills Symbols themselves (in their entirety). This works:

if (!('description' in Symbol.prototype)) {
  Object.defineProperty(Symbol.prototype, 'description', {
    get: function get() {
      return /\((.+)\)/.exec(this.toString())[1]
    },
  })
}


/**
* Available in:
* Edge: never
* Firefox: 62
* Chrome: 69
* Safari: 12
*
* https://caniuse.com/array-flat
*/
// Copied from https://gist.github.com/developit/50364079cf0390a73e745e513fa912d9
// Licensed Apache-2.0
if (!Array.prototype.flat) {
Array.prototype.flat = function flat(d, c) {
return (
(c = this.concat.apply([], this)),
d > 1 && c.some(Array.isArray) ? c.flat(d - 1) : c
)
}
Array.prototype.flatMap = function (c, a) {
return this.map(c, a).flat()
}
}

/**
* Available in:
* Edge: 18
* Firefox: 58
* Chrome: 63
* Safari: 11.1
*
* https://caniuse.com/promise-finally
*/
// Modified from https://gist.github.com/developit/e96097d9b657f2a2f3e588ffde433437
// Licensed Apache-2.0
if (!Promise.prototype.finally) {
Promise.prototype.finally = function (callback) {
if (typeof callback !== 'function') {
return this.then(callback, callback)
}

var P = this.constructor || Promise
return this.then(
function (value) {
return P.resolve(callback()).then(function () {
return value
})
},
function (err) {
return P.resolve(callback()).then(function () {
throw err
})
}
)
}
}
9 changes: 3 additions & 6 deletions packages/next/client/index.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
/* global location */
import '@next/polyfill-module'
import React from 'react'
import ReactDOM from 'react-dom'
import { HeadManagerContext } from '../next-server/lib/head-manager-context'
import mitt from '../next-server/lib/mitt'
import { RouterContext } from '../next-server/lib/router-context'
import { delBasePath, hasBasePath } from '../next-server/lib/router/router'
import type Router from '../next-server/lib/router/router'
import type {
AppComponent,
AppProps,
PrivateRouteInfo,
} from '../next-server/lib/router/router'
import { delBasePath, hasBasePath } from '../next-server/lib/router/router'
import { isDynamicRoute } from '../next-server/lib/router/utils/is-dynamic'
import * as querystring from '../next-server/lib/router/utils/querystring'
import * as envConfig from '../next-server/lib/runtime-config'
import { getURL, loadGetInitialProps, ST } from '../next-server/lib/utils'
import type { NEXT_DATA } from '../next-server/lib/utils'
import { getURL, loadGetInitialProps, ST } from '../next-server/lib/utils'
import initHeadManager from './head-manager'
import PageLoader, { looseToArray, StyleSheetTuple } from './page-loader'
import measureWebVitals from './performance-relayer'
Expand All @@ -41,10 +42,6 @@ declare global {
type RenderRouteInfo = PrivateRouteInfo & { App: AppComponent }
type RenderErrorProps = Omit<RenderRouteInfo, 'Component' | 'styleSheets'>

if (!('finally' in Promise.prototype)) {
;(Promise.prototype as PromiseConstructor['prototype']).finally = require('next/dist/build/polyfills/finally-polyfill.min')
}

const data: typeof window['__NEXT_DATA__'] = JSON.parse(
document.getElementById('__NEXT_DATA__')!.textContent!
)
Expand Down
2 changes: 1 addition & 1 deletion packages/next/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
"@babel/preset-typescript": "7.10.4",
"@babel/runtime": "7.11.2",
"@babel/types": "7.11.5",
"@next/polyfill-module": "9.5.4-canary.16",
"@next/react-dev-overlay": "9.5.4-canary.16",
"@next/react-refresh-utils": "9.5.4-canary.16",
"ast-types": "0.13.2",
Expand Down Expand Up @@ -179,7 +180,6 @@
"escape-string-regexp": "2.0.0",
"etag": "1.8.1",
"file-loader": "6.0.0",
"finally-polyfill": "0.1.0",
"find-up": "4.1.0",
"fresh": "0.5.2",
"gzip-size": "5.1.1",
Expand Down
14 changes: 1 addition & 13 deletions packages/next/taskfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,14 @@ export async function next__polyfill_nomodule(task, opts) {
.target('dist/build/polyfills')
}

export async function finally_polyfill(task, opts) {
await task
.source(
opts.src || relative(__dirname, require.resolve('finally-polyfill'))
)
.target('dist/build/polyfills')
}

export async function unfetch(task, opts) {
await task
.source(opts.src || relative(__dirname, require.resolve('unfetch')))
.target('dist/build/polyfills')
}

export async function browser_polyfills(task) {
await task.parallel([
'next__polyfill_nomodule',
'finally_polyfill',
'unfetch',
])
await task.parallel(['next__polyfill_nomodule', 'unfetch'])
}

const externals = {
Expand Down
8 changes: 4 additions & 4 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,17 @@ describe('Build Output', () => {
expect(parseFloat(indexSize) - 265).toBeLessThanOrEqual(0)
expect(indexSize.endsWith('B')).toBe(true)

// should be no bigger than 60.2 kb
expect(parseFloat(indexFirstLoad) - 60.5).toBeLessThanOrEqual(0)
// should be no bigger than 60.8 kb
expect(parseFloat(indexFirstLoad) - 60.8).toBeLessThanOrEqual(0)
expect(indexFirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0)
expect(err404Size.endsWith('kB')).toBe(true)

expect(parseFloat(err404FirstLoad) - 63.7).toBeLessThanOrEqual(0)
expect(parseFloat(err404FirstLoad) - 63.8).toBeLessThanOrEqual(0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll) - 60.2).toBeLessThanOrEqual(0)
expect(parseFloat(sharedByAll) - 60.4).toBeLessThanOrEqual(0)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7545,11 +7545,6 @@ finalhandler@~1.1.2:
statuses "~1.5.0"
unpipe "~1.0.0"

finally-polyfill@0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/finally-polyfill/-/finally-polyfill-0.1.0.tgz#2a17b16581d9477db16a703c7b79a898ac0b7d50"
integrity sha512-J1LEcZ5VXe1l3sEO+S//WqL5wcJ/ep7QeKJA6HhNZrcEEFj0eyC8IW3DEZhxySI2bx3r85dwAXz+vYPGuHx5UA==

find-cache-dir@3.3.1, find-cache-dir@^3.0.0, find-cache-dir@^3.3.1:
version "3.3.1"
resolved "https://registry.yarnpkg.com/find-cache-dir/-/find-cache-dir-3.3.1.tgz#89b33fad4a4670daa94f855f7fbe31d6d84fe880"
Expand Down