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

chore: update babel's present-env configuration #1434

Closed
wants to merge 5 commits into from
Closed

Conversation

sohkai
Copy link
Contributor

@sohkai sohkai commented Jun 3, 2020

See babel/babel#11451.

While this is not a problem locally because we have a yarn.lock, it breaks users who try to install using npm (e.g. users of the CLI).

Without this change, we get errors like:

If you'd like to use two separate instances of a plugin,
they need separate names, e.g.

  plugins: [
    ['some-plugin', {}],
    ['some-plugin', {}, 'some unique name'],
  ]

Duplicates detected are:
[
  {
    "alias": "/Users/brett/Development/aragon/aragon/node_modules/@babel/plugin-proposal-class-properties/lib/index.js",
    "dirname": "/Users/brett/Development/aragon/aragon",
    "ownPass": false,
    "file": {
      "request": "@babel/plugin-proposal-class-properties",
      "resolved": "/Users/brett/Development/aragon/aragon/node_modules/@babel/plugin-proposal-class-properties/lib/index.js"
    }
  },
  {
    "alias": "base$3",
    "options": {
      "loose": "#__internal__@babel/preset-env__prefer-false-but-true-is-ok-if-it-prevents-an-error"
    },
    "dirname": "/Users/brett/Development/aragon/aragon",
    "ownPass": false
  }
]
    at assertNoDuplicates (/Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/config-descriptors.js:206:13)
    at createDescriptors (/Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/config-descriptors.js:114:3)
    at createPluginDescriptors (/Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/config-descriptors.js:105:10)
    at /Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/config-descriptors.js:63:53
    at cachedFunction (/Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/caching.js:62:27)
    at cachedFunction.next (<anonymous>)
    at evaluateSync (/Users/brett/Development/aragon/aragon/node_modules/gensync/index.js:244:28)
    at sync (/Users/brett/Development/aragon/aragon/node_modules/gensync/index.js:84:14)
    at plugins (/Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/config-descriptors.js:28:77)
    at mergeChainOpts (/Users/brett/Development/aragon/aragon/node_modules/@babel/core/lib/config/config-chain.js:319:26)

@sohkai sohkai requested a review from bpierre June 3, 2020 15:04
@sohkai
Copy link
Contributor Author

sohkai commented Jun 3, 2020

NOTE: pins the yarn.lock's web3 version to 1.2.6, as with 1.2.7+, we start erroring due to the upgraded websocket dependency.

Turns out this wasn't the problem, and we can leave it unpinned.

@@ -6,13 +6,13 @@
"modules": false,
"useBuiltIns": "entry",
"core-js": 3,
"shippedProposals": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

}
],
"@babel/preset-react"
],
"plugins": [
["styled-components", { "displayName": true }],
"@babel/plugin-proposal-class-properties"
Copy link
Contributor

@bpierre bpierre Jun 3, 2020

Choose a reason for hiding this comment

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

Just wondering if babel was complaining without the shippedProposals: true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly didn't check, but just ruled it out of my mind since it was behaving differently between yarn and npm at that point after I initially did the yarn upgrade-interactive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went back to the commit and checked; this was not the problem. The problem was we had multiple versions of babel installed, and I believe an older one was still being used by parcel at that time.

Copy link
Contributor

Choose a reason for hiding this comment

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

what is shippedProposals?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rperez89 that’s an option of @babel/preset-env to enable ES features that are shipped in browsers, regardless of their TC39 stage: https://github.com/babel/website/blob/0d482dd7ed3451592549009483423f7d37d2fe92/docs/preset-env.md#shippedproposals

package.json Outdated Show resolved Hide resolved
"lint-staged": "^8.1.1",
"parcel-bundler": "^1.10.1",
"parcel-bundler": "^1.12.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out yarn upgrade-interactive doesn't do a great job of deduping.

Doing a yarn remove and then yarn add to re-install these dependencies (the only ones that had babel dependencies) properly dedupes the babel versions and makes the build work.

However, I cannot figure out why the websocket module's require() is still failing tests. Even going back to jest v25.1.0 and pushing everything else to be the same as master, it still fails now 🤷‍♂️. Perhaps also a good sign that we should not be creating a websocket connection in our tests :).

@sohkai sohkai marked this pull request as ready for review July 6, 2020 21:11
@auto-assign auto-assign bot requested a review from bpierre July 6, 2020 21:13
@stale stale bot added the abandoned label Aug 5, 2020
@aragon aragon deleted a comment from stale bot Aug 5, 2020
@stale stale bot removed the abandoned label Aug 5, 2020
@stale stale bot added the abandoned label Sep 5, 2020
@aragon aragon deleted a comment from stale bot Sep 5, 2020
@stale stale bot removed the abandoned label Sep 5, 2020
@stale stale bot added the abandoned label Oct 7, 2020
@aragon aragon deleted a comment from stale bot Oct 7, 2020
@stale stale bot removed the abandoned label Oct 7, 2020
@stale stale bot added the abandoned label Nov 7, 2020
@aragon aragon deleted a comment from stale bot Nov 7, 2020
@stale stale bot removed the abandoned label Nov 7, 2020
@sohkai
Copy link
Contributor Author

sohkai commented Nov 9, 2020

Going to let this close for now, we can upgrade this setting at a later time if necessary (since this was only for the dev tools to work well).

@sohkai sohkai closed this Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants