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

Enable corejs option for core bundle bin-prettier.js and third-party.js #6893

Closed
wants to merge 2 commits into from

Conversation

fisker
Copy link
Sponsor Member

@fisker fisker commented Nov 9, 2019

this PR requires #6866 , fixes #6891 , I tried to make a test for it, but failed. local test

before

$ cat index.js | node node_modules/prettier/bin-prettier --stdin --parser babel
[error] regeneratorRuntime is not defined

after

$ cat index.js | node dist/bin-prettier --stdin --parser babel
"use strict";

module.exports = require("./src/index");

diff

npm notice
-npm notice package: prettier@1.19.0
+npm notice package: prettier@1.20.0-dev
npm notice === Tarball Contents ===
npm notice 468B    package.json
-npm notice 1.4MB   bin-prettier.js
+npm notice 1.6MB   bin-prettier.js
npm notice 70.6kB  doc.js
npm notice 1.3MB   index.js
npm notice 1.1kB   LICENSE
npm notice 57.4kB  parser-angular.js
npm notice 266.2kB parser-babylon.js
npm notice 1.5MB   parser-flow.js
npm notice 139.6kB parser-glimmer.js
npm notice 51.4kB  parser-graphql.js
npm notice 110.7kB parser-html.js
npm notice 241.1kB parser-markdown.js
npm notice 351.8kB parser-postcss.js
npm notice 2.6MB   parser-typescript.js
npm notice 167.7kB parser-yaml.js
npm notice 3.7kB   README.md
npm notice 1.1MB   standalone.js
-npm notice 146.7kB third-party.js
+npm notice 296.0kB third-party.js
npm notice === Tarball Details ===
npm notice name:          prettier
-npm notice version:       1.19.0
+npm notice version:       1.20.0-dev
-npm notice filename:      prettier-1.19.0.tgz
+npm notice filename:      prettier-1.20.0-dev.tgz
-npm notice package size:  2.2 MB
+npm notice package size:  2.3 MB
-npm notice unpacked size: 9.6 MB
+npm notice unpacked size: 9.9 MB
-npm notice shasum:        0a355225fa968b1a716739508f2689d18a0a513a
+npm notice shasum:        5dc617bfb5759264b838f9d34d238578b220a850
-npm notice integrity:     sha512-p32VAB78i7JQT[...]txg82TWSxTi1A==
+npm notice integrity:     sha512-DLkijM5TPo6uU[...]PoR2Dnm5WbNGw==
npm notice total files:   18
npm notice
-prettier-1.19.0.tgz
+prettier-1.20.0-dev.tgz
  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@fisker fisker marked this pull request as ready for review November 9, 2019 05:55
@fisker fisker changed the title Enable corejs for core bin-prettier.js and third-party.js Enable corejs option for core bundle bin-prettier.js and third-party.js Nov 9, 2019
lydell added a commit that referenced this pull request Nov 9, 2019
Closes #6891.

This downgrades the get-stream package from 5.x to 4.x, which does not
use `async` functions which works around the problem of
`regeneratorRuntime` not being defined (it shouldn’t be needed).

This is an alternative solution to #6893 because I don’t know yet if we
want to enable `corejs` given that we will most likely drop support for
Node.js 8 and older in the next version.

Just like #6893 this PR comes without tests, but I verified locally that
`--stdin` works after this change.
@fisker fisker mentioned this pull request Nov 9, 2019
4 tasks
@lydell
Copy link
Member

lydell commented Nov 9, 2019

As far as I understand, we won’t need this in 2.0 so I’m closing. Let me know if I’ve misunderstood anything.

Closing in favor of #6894.

@lydell lydell closed this Nov 9, 2019
lydell added a commit that referenced this pull request Nov 9, 2019
Closes #6891.

This downgrades the get-stream package from 5.x to 4.x, which does not
use `async` functions which works around the problem of
`regeneratorRuntime` not being defined (it shouldn’t be needed).

This is an alternative solution to #6893 because I don’t know yet if we
want to enable `corejs` given that we will most likely drop support for
Node.js 8 and older in the next version.

Just like #6893 this PR comes without tests, but I verified locally that
`--stdin` works after this change.
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Feb 8, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 8, 2020
@fisker fisker deleted the enable-corejs branch December 5, 2022 05:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.19 Regression with --stdin
2 participants