Add missing es7.promise.finally polyfill when using useBuiltIns: usage by jsnajdr · Pull Request #8500 · babel/babel

jsnajdr · 2018-08-21T09:23:56Z

Usage of a finally instance method should trigger import of the es7.promise.finally
polyfill, but it doesn't. This PR adds the missing definition and a test.

Q	A
Fixed Issues?	#8297
Patch: Bug Fix?	Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

Usage of a `finally` instance method should trigger import of the `es7.promise.finally` polyfill, but it doesn't. This PR adds the missing definition and a test.

babel-bot · 2018-08-21T09:30:47Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8865/

zloirock · 2018-08-27T19:48:01Z

This way of adding Promise#finally could cause too many problems. If you at first use .finally method (for example, from non-promise object or system API) and after that Promise constructor, at first will be added Promise#finally and after that Promise constructor will be replaced to polyfilled Promise constructor without .finally.

zloirock · 2018-08-27T19:50:19Z

You will have this problem even with the order of import from fixtures from this PR.

existentialism · 2018-08-27T20:44:00Z

@zloirock thanks, I was concerned about this too =/

jsnajdr · 2018-08-28T10:06:43Z

Hello @zloirock and @existentialism , is there any way how this bug can be fixed?

I tried to add the Promise polyfills to the list of required polyfills for finally:

instanceMethods: {
  finally: ["es6.object.to-string", "es6.promise", "es7.promise.finally"],
}

but it didn't change the behavior of the promise-finally test at all.

I also don't understand why the test output has the reversed order of imports:

import "core-js/modules/es7.promise.finally";
import "core-js/modules/es6.promise";

In the transpiled source, the Promise identifier is encountered first and finally is the second. So what determines the order of the imports?

oles · 2018-08-28T13:55:47Z

I'm having an issue with this. Drilled it down to this:

const nothing = function() {}

const success = new Promise(nothing).finally()

const fail = new Promise(function() {
    new Promise(nothing).finally()
})

If I comment out success, it seems like es7.promise.finally is not included, as Firefox (version 61) yells .finally is not a function

@zloirock this is perhaps a concrete example of what you said?

zloirock · 2018-08-30T09:54:45Z

@oles nope.

zloirock · 2018-08-30T09:56:56Z

@jsnajdr IIRC preset-env add required methods in backward order.

mnunes01 · 2019-01-31T10:10:54Z

Hi, is this solved in anyway? i was testing and .finally was only working in Chrome and Safari.
Edge and firefox didn't work.

with the inclusion of "useBuiltIns: 'usage'," the error disappeared on Edge, but it failed silently and caused script execution problems. I had to remove finally from my code.

this.correlations[id].promise = new Promise((resolve, reject) => {
			this.correlations[id].resolve = resolve
			this.correlations[id].reject = reject
		})
		/*.finally(()=> {
			this.clearCorrelation(id) 
		})*/

wepack.dist.config.js:

var config = {
	mode: 'production',
	devtool: 'source-map',
	module: {
		rules: [
			{
				test: /\.js?$/,
				exclude: /node_modules/,
				loader: 'babel-loader',
				query: {
					presets: [ ['@babel/preset-env', {
						'targets': {
							'browsers': [
								'> 0.3%',
								'not dead',
								'not IE 11',
								'not IE_Mob 11'								
							]
						},
						useBuiltIns: 'usage',
					}] ],						
					plugins: [ '@babel/plugin-transform-runtime' ]
				}
			},
			{
				test: /\.css$/,
				use: [
					{
						loader: 'style-loader'
					},
					{
						loader: 'css-loader',
						options: {
							importLoaders: 1
						}
					}
				]
			}
		]
	},
	entry: [ 'whatwg-fetch','./src/index.js' ],
	output: {
		filename: 'js/output.js',
		path: path.resolve(__dirname, 'dist/publicApi'),
		libraryTarget: 'this'
	},
	plugins: [
		new webpack.DefinePlugin({
			'process.env': {
				NODE_ENV: JSON.stringify('production'),
				PLUGINS_PATH: JSON.stringify('plugins/@myplugins/'),
				VERSION: JSON.stringify(originalPackage.version),
				DEBUG: false,
				MONITOR_DEBUG_LOG_LEVEL: 1 //0, 1, 2, 3
			}
		}),
		new UglifyJSPlugin({
			sourceMap: true
		}),
		new GeneratePackageJsonPlugin({...configPackage, ...commonConfig}, versionsPackageFilename)
	]
}

package.json:

"devDependencies": {
    "@babel/core": "^7.0.0",
    "@babel/plugin-transform-runtime": "^7.0.0",
    "@babel/preset-env": "^7.0.0",
    "@babel/runtime": "^7.3.1",
    "babel-loader": "^8.0.0",
    "copy-webpack-plugin": "^4.6.0",
    "css-loader": "^2.1.0",
    "docdash": "^1.0.2",
    "eslint": "^4.19.1",
    "generate-package-json-webpack-plugin": "^1.0.0",
    "jsdoc": "^3.5.5",
    "nyc": "^11.7.1",
    "style-loader": "^0.23.1",
    "uglifyjs-webpack-plugin": "^2.1.1",
    "webpack": "^4.29.0",
    "webpack-bundle-analyzer": "^2.12.0",
    "webpack-cli": "^3.2.1",
    "webpack-dev-server": "^3.1.14",
    "writefile": "^0.2.8"
  },

ArmorDarks · 2019-05-19T14:49:20Z

Was it released?

nicolo-ribaudo · 2019-05-19T16:52:25Z

Yeah, since 7.0.0

Add missing es7.promise.finally polyfill when using useBuiltIns: usage

ea1d29a

Usage of a `finally` instance method should trigger import of the `es7.promise.finally` polyfill, but it doesn't. This PR adds the missing definition and a test.

nicolo-ribaudo approved these changes Aug 21, 2018

View reviewed changes

hzoo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Aug 21, 2018

hzoo merged commit 8874c5c into babel:master Aug 21, 2018

existentialism mentioned this pull request Aug 21, 2018

@babel/preset-env fails to polyfill Promise#finally #8297

Closed

jsnajdr deleted the usage-polyfill-promise-finally branch August 22, 2018 12:20

jsnajdr mentioned this pull request Aug 27, 2018

Packages: Change how required built-ins are polyfilled with Babel 7 WordPress/gutenberg#9171

Merged

6 tasks

mvollmer mentioned this pull request Oct 1, 2018

lib, apps: Provide polyfill for Promise.finally cockpit-project/cockpit#10178

Merged

alexsegura mentioned this pull request Jul 17, 2019

Cannot use Promise.finally on Firefox coopcycle/coopcycle-web#624

Closed

lock Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 3, 2019

lock Bot locked as resolved and limited conversation to collaborators Oct 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add missing es7.promise.finally polyfill when using useBuiltIns: usage#8500

Add missing es7.promise.finally polyfill when using useBuiltIns: usage#8500
hzoo merged 1 commit intobabel:masterfrom
jsnajdr:usage-polyfill-promise-finally

jsnajdr commented Aug 21, 2018 •

edited

Loading

Uh oh!

babel-bot commented Aug 21, 2018

Uh oh!

zloirock commented Aug 27, 2018

Uh oh!

zloirock commented Aug 27, 2018

Uh oh!

existentialism commented Aug 27, 2018

Uh oh!

jsnajdr commented Aug 28, 2018

Uh oh!

oles commented Aug 28, 2018

Uh oh!

zloirock commented Aug 30, 2018

Uh oh!

zloirock commented Aug 30, 2018

Uh oh!

mnunes01 commented Jan 31, 2019 •

edited

Loading

Uh oh!

ArmorDarks commented May 19, 2019

Uh oh!

nicolo-ribaudo commented May 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Uh oh!

Conversation

jsnajdr commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Aug 21, 2018

Uh oh!

zloirock commented Aug 27, 2018

Uh oh!

zloirock commented Aug 27, 2018

Uh oh!

existentialism commented Aug 27, 2018

Uh oh!

jsnajdr commented Aug 28, 2018

Uh oh!

oles commented Aug 28, 2018

Uh oh!

zloirock commented Aug 30, 2018

Uh oh!

zloirock commented Aug 30, 2018

Uh oh!

mnunes01 commented Jan 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArmorDarks commented May 19, 2019

Uh oh!

nicolo-ribaudo commented May 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

jsnajdr commented Aug 21, 2018 •

edited

Loading

mnunes01 commented Jan 31, 2019 •

edited

Loading