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

Quick-fix for stdin being broken in 1.19.0 #6894

Merged
merged 2 commits into from Nov 9, 2019
Merged

Quick-fix for stdin being broken in 1.19.0 #6894

merged 2 commits into from Nov 9, 2019

Conversation

@lydell
Copy link
Collaborator

lydell commented 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.

/cc @fisker @evilebottnawi

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

This comment has been minimized.

Copy link
Contributor

fisker commented Nov 9, 2019

Do you want me send a pr transform async to promies? I can do that

@fisker fisker mentioned this pull request Nov 9, 2019
1 of 4 tasks complete
@lydell

This comment has been minimized.

Copy link
Collaborator Author

lydell commented Nov 9, 2019

@fisker So my thinking is – once we start working on 2.0 we want to remove all of these things from the build script, right? So wouldn’t it be easier to just downgrade get-stream? Then we don’t have to clean up as much later.

@fisker

This comment has been minimized.

Copy link
Contributor

fisker commented Nov 9, 2019

@lydell

I'm thinking differently, instead of removing things, we should let babel(mostly corejs) to take care of any syntax problem.

Yes, we drop old version of node support, but node 10 will be old someday, we don't want start adding thing again.

@lydell

This comment has been minimized.

Copy link
Collaborator Author

lydell commented Nov 9, 2019

Let’s take that decision when Node.js 10 is old (in a year or so).

@fisker

This comment has been minimized.

Copy link
Contributor

fisker commented Nov 9, 2019

Are we going to remove babel, when we start 2.0?

@lydell

This comment has been minimized.

Copy link
Collaborator Author

lydell commented Nov 9, 2019

I don’t know yet.

@lydell lydell merged commit 9b1726d into master Nov 9, 2019
18 checks passed
18 checks passed
Header rules No header rules processed
Details
Pages changed All files already uploaded
Details
Mixed content No mixed content detected
Details
Redirect rules 4 redirect rules processed
Details
codecov/patch Coverage not affected when comparing 8c3efeb...c0ac812
Details
codecov/project 94.48% remains the same compared to 8c3efeb
Details
deploy/netlify Deploy preview ready!
Details
prettier.prettier Build #20191109.25 succeeded
Details
prettier.prettier (Dev Lint on Linux Node v12) Dev Lint on Linux Node v12 succeeded
Details
prettier.prettier (Dev Test on Linux Node v12) Dev Test on Linux Node v12 succeeded
Details
prettier.prettier (Dev Test on Windows Node v12) Dev Test on Windows Node v12 succeeded
Details
prettier.prettier (Dev Test on macOS Node v12) Dev Test on macOS Node v12 succeeded
Details
prettier.prettier (Prod Build on Linux Node v12) Prod Build on Linux Node v12 succeeded
Details
prettier.prettier (Prod Lint on Linux Node v12) Prod Lint on Linux Node v12 succeeded
Details
prettier.prettier (Prod Pack on Linux Node v12) Prod Pack on Linux Node v12 succeeded
Details
prettier.prettier (Prod Test on macOS Node_v12) Prod Test on macOS Node_v12 succeeded
Details
prettier.prettier (Prod Test on macOS Node_v12_standalone) Prod Test on macOS Node_v12_standalone succeeded
Details
prettier.prettier (Prod Test on macOS Node_v4) Prod Test on macOS Node_v4 succeeded
Details
@lydell lydell deleted the stdin-quick-fix branch Nov 9, 2019
@fisker

This comment has been minimized.

Copy link
Contributor

fisker commented Nov 9, 2019

I really dont think we can , for this issue I'm okay with this solution. But #6866 is only a option, I add it to prepare for some deps like this, we can't always go back, some deps might requires lot of changes, we can use corejs just in case, please keep #6866 open

@lydell

This comment has been minimized.

Copy link
Collaborator Author

lydell commented Nov 9, 2019

Sorry, I don’t understand what you mean.

@fisker

This comment has been minimized.

Copy link
Contributor

fisker commented Nov 9, 2019

please reopen #6866, we can discuss there

lipis added a commit that referenced this pull request Nov 12, 2019
* 'master' of github.com:prettier/prettier: (31 commits)
  Bump jest-watch-typeahead from 0.4.0 to 0.4.2 (#6923)
  Bump unified from 8.4.1 to 8.4.2 (#6927)
  refactoring: Babel's error recovery superseded option combinations (#6930)
  Update `fsevents` in yarn.lock (#6909)
  Run CI on the `next` branch
  Fix bin permissions (#6902)
  Add missing headings to changelog
  Fix code block in changelog
  Bump Prettier dependency to 1.19.1
  Release 1.19.1
  Quick-fix for stdin being broken in 1.19.0 (#6894)
  Fix `since` version for `vueIndentScriptAndStyle` (#6897)
  Remove out-of-date comment
  fix formatting of union type as arrow function return type (#6896)
  Try to fix some code blocks in 1.19.0 blog post
  Blog post, changelog and docs for 1.19 (#6787)
  Bump Prettier dependency to 1.19.0
  Release 1.19.0
  prettier 1.19.0-beta.1
  deduplicate entries in yarn.lock - part 2 (#6884)
  ...
@fisker fisker mentioned this pull request Jan 16, 2020
1 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.