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

[jsnajdr]

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

[babel-bot]

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

[zloirock]

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]

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

[existentialism]

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

[jsnajdr]

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]

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]

@oles nope.

[zloirock]

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

[mnunes01]

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]

Was it released?

[nicolo-ribaudo]

Yeah, since 7.0.0