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

objectSpread2 failure #10192

Closed
XhmikosR opened this issue Jul 10, 2019 · 4 comments · Fixed by #10189
Closed

objectSpread2 failure #10192

XhmikosR opened this issue Jul 10, 2019 · 4 comments · Fixed by #10189
Labels
i: bug i: needs triage outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@XhmikosR
Copy link

XhmikosR commented Jul 10, 2019

Bug Report

Current Behavior

While trying to update babel and its plugins to the latest v7.5.4 version, we still get some failures in our test suite https://travis-ci.org/twbs/bootstrap/jobs/556685620#L788

The related code should be https://github.com/twbs/bootstrap/blob/master/js/src/dom/manipulator.js#L47

Going back to v7.4.x and objectSpread works as expected.

Input Code

Babel Configuration (.babelrc, package.json, cli command)

module.exports = {
  presets: [
    [
      '@babel/env',
      {
        loose: true,
        modules: false,
        exclude: ['transform-typeof-symbol']
      }
    ]
  ],
  plugins: [
    '@babel/plugin-proposal-object-rest-spread'
  ],
  env: {
    test: {
      plugins: [ 'istanbul' ]
    }
  }
};

Environment

  • Babel version(s): v7.5.4
  • Node/npm version: Node 10.16.0/npm 6.9.0
  • OS: Windows 10
  • Monorepo: no
  • How you are using Babel: cli
@babel-bot
Copy link
Collaborator

Hey @XhmikosR! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@JLHwung
Copy link
Contributor

JLHwung commented Jul 10, 2019

The unit test fails after Object.getOwnPropertyDescriptor is stub to return false.

Since 7.5.0, the objectSpread will call Object.getOwnPropertyDescriptor under the hood to filter enumerable properties. However, since most of time a DOMStringMap would not include a symbol key and Object.keys() always return enumerable string properties, objectSpread should not invoke Object.getOwnPropertyDescriptor.

Though it seems quite irrelevant on the surface. This issue would be fixed by #10189.

A workaround will be to remove the stub and wait until #10189 is released. After it is released it should be okay to keep the stub.

@nicolo-ribaudo
Copy link
Member

Note: while I'm happy that this issue has been fixed for your use case, unfortunately Babel can't guarantee that it works (or will work) when built-in functions have been modified in a nonstandard way.

@Johann-S
Copy link

Thanks @JLHwung it was because of our unit tests nothings related to Babel here 👍

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: bug i: needs triage outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants