Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.

Conversation

@thibaudcolas
Copy link
Contributor

@thibaudcolas thibaudcolas commented Feb 16, 2018

Summary

This dependency got introduced in a separate commit (f8ca29d) from the others in #1644. I imagine this is a mistake since it only provides dev-time tooling.

Test Plan

For development, we can try a fresh install of the project:

  1. rm -rf node_modules
  2. yarn install. This runs npm run build via the prepublish script, so if it works it's good to go.

I don't think dependencies and devDependencies are processed any differently in a development environment, so not sure this check is even necessary.

For the published package:

  1. mkdir test
  2. npm init -y
  3. npm install git+https://github.com/thibaudcolas/draft-js.git#patch-2
  4. npm ls --depth=1

The npm ls result should be:

├─┬ draft-js@0.10.5 (git+https://github.com/thibaudcolas/draft-js.git#cb06920d0f3dea77692680dddba6ec475663c7b2)
│ ├── fbjs@0.8.16
│ ├── immutable@3.7.6
│ └── object-assign@4.1.1
├── UNMET PEER DEPENDENCY react@^0.14.0 || ^15.0.0-rc || ^16.0.0-rc || ^16.0.0
└── UNMET PEER DEPENDENCY react-dom@^0.14.0 || ^15.0.0-rc || ^16.0.0-rc || ^16.0.0

Note uglifyjs-webpack-plugin is absent, whereas it would have been there with npm install git+https://github.com/thibaudcolas/draft-js.git#master.

This dependency got introduced in a separate commit (f8ca29d) from the others in facebookarchive#1644. I imagine this is a mistake since it only provides dev-time tooling.
@mitermayer
Copy link
Contributor

Thanks for this PR!

Copy link
Contributor

@flarnie flarnie left a comment

Choose a reason for hiding this comment

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

Thanks! Will add to my queue for merging.

small_shiny_jake_happy

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@flarnie is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@flarnie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@thibaudcolas thibaudcolas deleted the patch-2 branch February 26, 2018 19:56
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
Summary:
**Summary**

This dependency got introduced in a separate commit (f8ca29d1a7fa5a8bb89cb9be9a66d12732a0bfa7) from the others in #1644. I imagine this is a mistake since it only provides dev-time tooling.

**Test Plan**

For development, we can try a fresh install of the project:

1. `rm -rf node_modules`
2. `yarn install`. This runs `npm run build` via the `prepublish` script, so if it works it's good to go.

I don't think dependencies and devDependencies are processed any differently in a development environment, so not sure this check is even necessary.

For the published package:

1. `mkdir test`
2. `npm init -y`
3. `npm install git+https://github.com/thibaudcolas/draft-js.git#patch-2`
4. `npm ls --depth=1`

The `npm ls` result should be:

```
├─┬ draft-js@0.10.5 (git+https://github.com/thibaudcolas/draft-js.git#cb06920d0f3dea77692680dddba6ec475663c7b2)
│ ├── fbjs@0.8.16
│ ├── immutable@3.7.6
│ └── object-assign@4.1.1
├── UNMET PEER DEPENDENCY react@^0.14.0 || ^15.0.0-rc || ^16.0.0-rc || ^16.0.0
└── UNMET PEER DEPENDENCY react-dom@^0.14.0 || ^15.0.0-rc || ^16.0.0-rc || ^16.0.0
```

Note `uglifyjs-webpack-plugin` is absent, whereas it would have been there with `npm install git+https://github.com/thibaudcolas/draft-js.git#master`.
Closes facebookarchive/draft-js#1653

Differential Revision: D7073476

fbshipit-source-id: f69e31c2ca252062d2d66080055fd08b88c5a275
aforismesen added a commit to aforismesen/draft-js that referenced this pull request Jul 12, 2024
Summary:
**Summary**

This dependency got introduced in a separate commit (f8ca29d1a7fa5a8bb89cb9be9a66d12732a0bfa7) from the others in #1644. I imagine this is a mistake since it only provides dev-time tooling.

**Test Plan**

For development, we can try a fresh install of the project:

1. `rm -rf node_modules`
2. `yarn install`. This runs `npm run build` via the `prepublish` script, so if it works it's good to go.

I don't think dependencies and devDependencies are processed any differently in a development environment, so not sure this check is even necessary.

For the published package:

1. `mkdir test`
2. `npm init -y`
3. `npm install git+https://github.com/thibaudcolas/draft-js.git#patch-2`
4. `npm ls --depth=1`

The `npm ls` result should be:

```
├─┬ draft-js@0.10.5 (git+https://github.com/thibaudcolas/draft-js.git#cb06920d0f3dea77692680dddba6ec475663c7b2)
│ ├── fbjs@0.8.16
│ ├── immutable@3.7.6
│ └── object-assign@4.1.1
├── UNMET PEER DEPENDENCY react@^0.14.0 || ^15.0.0-rc || ^16.0.0-rc || ^16.0.0
└── UNMET PEER DEPENDENCY react-dom@^0.14.0 || ^15.0.0-rc || ^16.0.0-rc || ^16.0.0
```

Note `uglifyjs-webpack-plugin` is absent, whereas it would have been there with `npm install git+https://github.com/thibaudcolas/draft-js.git#master`.
Closes facebookarchive/draft-js#1653

Differential Revision: D7073476

fbshipit-source-id: f69e31c2ca252062d2d66080055fd08b88c5a275
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants