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

@babel/polyfill with useBuiltIns: 'usage' , web-dom-iterable polyfill unnecsessarily loaded #9671

Closed
gaurav5430 opened this Issue Mar 12, 2019 · 5 comments

Comments

Projects
None yet
3 participants
@gaurav5430
Copy link

gaurav5430 commented Mar 12, 2019

Bug Report

Current Behavior
A clear and concise description of the behavior.

I am using Object.keys to iterate over keys of an object, but the polyfill includes web-dom-iterable polyfill, which shouldn't be required in this case.

Input Code

  • REPL or Repo link if applicable:
import React from "react";
import PropTypes from "prop-types";

function getHeaders(data) {
  let headers = [];
  headers = Object.keys(data[0]);
  return headers;
}

function renderHeader(data) {
  const headers = getHeaders(data);
  return (
    <tr>
      {headers.map((header, index) => (
        // eslint-disable-next-line react/no-array-index-key
        <th className="table__header-cell" key={index}>
          {header}
        </th>
      ))}
    </tr>
  );
}

function renderRow(rowObj, data) {
  const headers = getHeaders(data);
  return (
    <tr key={rowObj[headers[0]]}>
      {headers.map((header, index) => (
        // eslint-disable-next-line react/no-array-index-key
        <td className="table__cell" key={index}>
          {rowObj[header]}
        </td>
      ))}
    </tr>
  );
}

function renderBody(data) {
  return data.map(item => renderRow(item, data));
}

function Table(props) {
  const { data } = props;
  return (
    <table className="table">
      <thead>{renderHeader(data)}</thead>
      <tbody className="table__body">{renderBody(data)}</tbody>
    </table>
  );
}

Table.propTypes = {
  data: PropTypes.arrayOf(PropTypes.object).isRequired
};

export default Table;

Expected behavior/code
A clear and concise description of what you expected to happen (or code).

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

const presets = [
  "@babel/react",
  [
    "@babel/env",
    {
      targets: {
        edge: "17",
        firefox: "60",
        chrome: "67",
        safari: "11.1"
      },
      useBuiltIns: "usage",
      loose: true
    }
  ]
];

const plugins = ["@babel/plugin-syntax-dynamic-import"];

module.exports = { presets, plugins };

Environment

  • Babel version(s): [7.3.4]
  • Node/npm version: [Node 8]
  • OS: [ Windows 10]
  • Monorepo: [e.g. yes/no/Lerna]
  • How you are using Babel: babel-loader with webpack

Possible Solution

Additional context/Screenshots
Add any other context about the problem here. If applicable, add screenshots to help explain.

I understand that though this is not expected behaviour, this might be hard to fix, as 'usage' option might not be able to guess correctly on whether or not a certain polyfill is required.
I am reporting this mostly for completeness, and for gathering information on how the polyfill actually concluded that it requires web-dom-iterable.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Mar 12, 2019

Hey @gaurav5430! 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.

@zloirock

This comment has been minimized.

Copy link
Member

zloirock commented Mar 12, 2019

The way of determination of source for this case was improved for core-js@3 target in #7646. It will be available after some days. I don't think that makes sense change this behavior for core-js@2 target which will be legacy.

@gaurav5430

This comment has been minimized.

Copy link
Author

gaurav5430 commented Mar 12, 2019

That's great.
Yeah it does not make sense to change this behavior for core-js@2 if it is going to be legacy.

I don't see a specific entry for Object.keys or web-dom-iterable on #7646 though, can you point to the comment where it is discussed?
Or are you suggesting that there are general improvements which should take care of this issue as well?

@zloirock

This comment has been minimized.

Copy link
Member

zloirock commented Mar 12, 2019

useBuilIns internal plugins were completely rewritten, see useBuilIns: usage with corejs: 3 section.

@zloirock

This comment has been minimized.

Copy link
Member

zloirock commented Mar 12, 2019

A added a simple test with Object.keys for your case.

@zloirock zloirock closed this Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.