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

Shouldn't loose-envify be in devDependencies instead of dependencies? #203

Closed
autra opened this issue Aug 8, 2018 · 7 comments
Closed

Comments

@autra
Copy link
Contributor

autra commented Aug 8, 2018

If I'm not mistaken, it's only used in the umd script in the package.json. If I understand correctly, this script builds a bundle for browsers, and will only be used in dev process.

This would remove unused transitive dependencies when using a package that depends on prop-types.

@ljharb
Copy link
Collaborator

ljharb commented Aug 8, 2018

Anything that’s needed in production, even conceptually, is a runtime dep - so the fact that it’s included in a bundle doesn’t mean it shouldn’t be installed as a dep.

@autra
Copy link
Contributor Author

autra commented Aug 8, 2018

Anything that’s needed in production, even conceptually, is a runtime dep

Of course, but it's not the case for loose-envify here, as it is used only by browserify (a dev deps itself btw). Or did I miss one usage?

But I'm not sure of what you mean:

the fact that it’s included in a bundle doesn’t mean it shouldn’t be installed as a dep.

you mean "the fact that it's not included in a bundle doesn't...", right? Just to be sure I understood correctly.

@ljharb
Copy link
Collaborator

ljharb commented Aug 8, 2018

ahhh gotcha, i misunderstood. Yes, if loose-envify is part of the build process then it should definitely be a dev dep.

@billyjanitsch
Copy link

@ljharb @autra the reason it was listed as a dependency rather than a dev dependency is because these lines tell the app level browserify to use loose-envify when processing the package (to remove the process.env.NODE_ENV switches). This doesn't happen at package build time because those switches need to be left in the published package. Many other packages, such as react-router, invariant, and react itself, have loose-envify as a production dependency for the same reason.

So prop-types@15.7.0 likely broke support for browserify. It would probably be better to do this in a major release, accompanied by the removal of the package.json lines that I linked.

@ljharb
Copy link
Collaborator

ljharb commented Feb 11, 2019

@billyjanitsch if that’s the case, we can revert it and put it back - can you help me by providing a quick test case to verify that it does indeed need to be a runtime dep?

@ljharb ljharb reopened this Feb 11, 2019
@autra
Copy link
Contributor Author

autra commented Feb 12, 2019

So prop-types@15.7.0 likely broke support for browserify. It would probably be better to do this in a major release, accompanied by the removal of the package.json lines that I linked.

Sorry for that! I didn't intent to remove browserify support though, so maybe it's better to simply revert it? I see no other way if we want to keep this support.

@ljharb ljharb closed this as completed in 32114d3 Feb 12, 2019
@ljharb
Copy link
Collaborator

ljharb commented Feb 13, 2019

v15.7.2 is released; please file an issue if you see any problems!

@ljharb ljharb mentioned this issue Apr 11, 2019
2 tasks
MichaelBuessemeyer added a commit to scalableminds/prop-types that referenced this issue May 17, 2024
* Include CI for master branch and PR's

In accordance with facebook#19,
includes config for TravisCI to run test reports on the master branch
and PR's to the master branch.

- Includes .travis.yml config file
- Includes current CI status badge in README

Closes facebook#19.

* Point readme to correct docs for production builds (facebook#153)

After an update to the docs, the production build was pointing to an outdated link. This commit directs the link to the appropriate location in the React docs.

* 15.6.1

* Add 15.6.1 to CHANGELOG

* Updated vars with consts and lets in PropTypesProductionReact15-test.js

* Updated vars to consts and lets in PropTypesDevelopmentReact15.js

* Updated vars to consts and lets in PropTypesDevelopmentStandalone-test.js

* Updated vars to consts and lets in PropTypesProductionStandalone-test.js

* Add example for `PropTypes.exact`

* Show that shapes can have required properties

* Move explanation of `isRequired` and show it in `PropTypes.shape`

* Remove trailing spaces

* Remove fbjs dependency

* Preserve "Invariant Violation" name

* 15.6.2

* .com

* Add support for objects with a null prototype in objectOf

* Replace `hasOwnProperty` with the more robust "has" package

* Revert "Replace `hasOwnProperty` with the more robust "has" package"

This reverts commit e6a9b28.

* Inline the `has` module

* missed semicolon

* [Tests] use componentName in getPropTypeWarningMessage instead of hard-coded testComponent

* [Fix] Fix `oneOf` when used with Symbols

- Fixes error due to an attempt to coerce a Symbol to a string
- Improves formatting of the "expected" portion of the generated
  warning, outputting for example `["Symbol(A)","Symbol(B)"]` rather
  than `[null,null]`

Fixes facebook#10

* Completely remove envify in favor of loose-envify

Closes facebook#203.

* [New] Add `.elementType`

* Add license to readme

* [Fix] Support validation when hasOwnProperty is not in prototype

Closes facebook#183; relates to facebook#112.

* [Docs] fix relative release date to be absolute

* [Docs] Fix typo in example

* [New] add `PropTypes.resetWarningCache`

* [dev deps] update `loose-envify`

* `oneOf`: improve warning when multiple arguments are supplied

Adds a different warning message for multiple arguments supplied to oneOf. A common mistake is to write oneOf(x, y, z) instead of oneOf([x, y, z]) and this should help developers identifying the error.

* [changelog] update repo links

* v15.7.0

* [Fix] avoid template literal syntax

Fixes facebook#255. Fixes facebook#254.

* v15.7.1

* [Tests] add `eslint`; run more travis tests

* [Tests] run tests with multiple react versions

* [Fix] move `loose-envify` back to production deps, for browerify usage

Fixes facebook#203

* [Tests] add additional passing tests

* [Fix] ensure nullish values in `oneOf` do not crash

Fixes facebook#256.

* [dev deps] update `browserify`

* v15.7.2

* [Docs] Improve wording for `checkPropTypes`

* [Docs] `PropTypes.node`: add link to react docs

Fixes facebook#154.

* [meta] use `in-publish` to avoid running the build on install

* `checkPropTypes`: Friendlier message when using a type checker that is not a function

* [Tests] test the build process

* [New] Add type check for validator for 'shape' and 'exact'

Fixes facebook#220.

* [Tests] fix broken tests

* [Refactor] extract `has`

* [Docs] Add instructions for intentional inclusion of validation in production.

* [New] `oneOfType`: Add expected types to warning

Adds data object to returned error in checker so that the expected types can be accessed from within oneOfType check. Also, updates the tests slightly.

Fixes facebook#9.

* [Tests] Fixed typo: 'Any type *should* accept any value'

* [Dev Deps] update `eslint`

* [Deps] update `react-is`

* [Dev Deps] update `browserify`, `bundle-collapser`, `react`, `uglifyify`, `uglifyjs`

* [Docs] Typo fix in example

Reverts an incorrect typo fix made in facebook#248.

Closes facebook#299

* [Tests] Fix spelling

* doc: highlighted the func name (facebook#321)

* [readme] Clarify usage of `elementType`

Closes facebook#334.

* [Dev Deps] update `bundle-collapser`, `eslint`, `in-publish`, `react`

* [Deps] update `react-is`

* Add a package `sideEffects` field.

* Bump sshpk from 1.13.1 to 1.16.1

Bumps [sshpk](https://github.com/joyent/node-sshpk) from 1.13.1 to 1.16.1.
- [Release notes](https://github.com/joyent/node-sshpk/releases)
- [Commits](TritonDataCenter/node-sshpk@v1.13.1...v1.16.1)

Signed-off-by: dependabot[bot] <support@github.com>

* Use GH Actions

This migrates to a more accessible platform

* [eslint] enable some rules

Additional ESLint rules: "no-multi-spaces": ["error"], "key-spacing": ["error"]
"no-multi-spaces" - Disallow multiple spaces;
"key-spacing" - Enforce consistent spacing between keys and values in object literal properties;

* Bump path-parse from 1.0.6 to 1.0.7

Bumps [path-parse](https://github.com/jbgutierrez/path-parse) from 1.0.6 to 1.0.7.
- [Release notes](https://github.com/jbgutierrez/path-parse/releases)
- [Commits](https://github.com/jbgutierrez/path-parse/commits/v1.0.7)

---
updated-dependencies:
- dependency-name: path-parse
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump tmpl from 1.0.4 to 1.0.5

Bumps [tmpl](https://github.com/daaku/nodejs-tmpl) from 1.0.4 to 1.0.5.
- [Release notes](https://github.com/daaku/nodejs-tmpl/releases)
- [Commits](https://github.com/daaku/nodejs-tmpl/commits/v1.0.5)

---
updated-dependencies:
- dependency-name: tmpl
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* [deps] regenerate yarn.lock

* [readme] Fix branch name (master -> main)

* [Dev Deps] update `eslint`

* [New] add `PropTypes.bigint`

Closes facebook#355

* v15.8.0

* [Tests] do not fail fast; add react 17

* [meta] Fix formatting in CHANGELOG.md

* [Tests] convert normal `it` functions to arrow functions

* [Tests] add missing test coverage

* [Fix] fix crash when a custom propType return lacks `.data`; call `hasOwnProperty` properly

Fixes facebook#369

* [Dev Deps] update `eslint`

* v15.8.1

* docs: add GH button in support of Ukraine (facebook#375)

## Summary
Our mission at Meta Open Source is to empower communities through open source, and we believe that it means building a welcoming and safe environment for all. As a part of this work, we are adding this banner in support for Ukraine during this crisis.

* [Deps] `yarn upgrade`

* build: harden ci.yml permissions

Signed-off-by: Alex <aleksandrosansan@gmail.com>

* Bump semver from 5.7.1 to 5.7.2

Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2.
- [Release notes](https://github.com/npm/node-semver/releases)
- [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md)
- [Commits](npm/node-semver@v5.7.1...v5.7.2)

---
updated-dependencies:
- dependency-name: semver
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump ua-parser-js from 0.7.32 to 0.7.33

Bumps [ua-parser-js](https://github.com/faisalman/ua-parser-js) from 0.7.32 to 0.7.33.
- [Release notes](https://github.com/faisalman/ua-parser-js/releases)
- [Changelog](https://github.com/faisalman/ua-parser-js/blob/master/changelog.md)
- [Commits](faisalman/ua-parser-js@0.7.32...0.7.33)

---
updated-dependencies:
- dependency-name: ua-parser-js
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* remove production env tests as we do not have any production env tests

* update

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Alex <aleksandrosansan@gmail.com>
Co-authored-by: Joe Fraley <joefraley@protonmail.com>
Co-authored-by: Quinn Stearns <quinn@datahost.com>
Co-authored-by: Brandon Dail <brandondail@fb.com>
Co-authored-by: Brandon Dail <aweary@users.noreply.github.com>
Co-authored-by: Barry <barry@iplatform.co.za>
Co-authored-by: Christian Paul <info@jaller.de>
Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
Co-authored-by: Julien Mourer <getkey@getkey.eu>
Co-authored-by: ksmolniy <k.smolniy@gmail.com>
Co-authored-by: C. T. Lin <chentsulin@gmail.com>
Co-authored-by: Jim Fitzpatrick <fitzpatrick.jim@gmail.com>
Co-authored-by: Augustin Trancart <augustin.trancart@oslandia.com>
Co-authored-by: Benoit Tremblay <trembl.ben@gmail.com>
Co-authored-by: Dominik Ferber <dominik.ferber@gmail.com>
Co-authored-by: Joseph A. Szczesniak <NukaPunk@users.noreply.github.com>
Co-authored-by: Troy Rhinehart <troy.rhinehart@gmail.com>
Co-authored-by: Patrick Way <patrick.way@intersection.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Wojciech Maj <kontakt@wojtekmaj.pl>
Co-authored-by: NoScripter <noscripter@users.noreply.github.com>
Co-authored-by: Gregory Desfour <gregory.desfour@accedo.tv>
Co-authored-by: Asbjørn Hegdahl <asbjorn.hegdahl@creuna.no>
Co-authored-by: NoScripter <mhz_xz_tc@126.com>
Co-authored-by: rgraffbrd <31221247+rgraffbrd@users.noreply.github.com>
Co-authored-by: Josh Alling <joshralling@gmail.com>
Co-authored-by: weiluntong <weiluntong.inbox@gmail.com>
Co-authored-by: Conrad Buck <conrad@plangrid.com>
Co-authored-by: Mark McCann <mail@markmccann.me>
Co-authored-by: John Bampton <jbampton@users.noreply.github.com>
Co-authored-by: Haseeb Khan <haseebasif97@gmail.com>
Co-authored-by: G Roques <groques360@gmail.com>
Co-authored-by: Jayden Seric <me@jaydenseric.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Paul O’Shannessy <paul@oshannessy.com>
Co-authored-by: Konstantin Popov <konst.hardy@gmail.com>
Co-authored-by: Dmitry Vinnik <dmitryvinn@users.noreply.github.com>
Co-authored-by: Alex <aleksandrosansan@gmail.com>
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

No branches or pull requests

3 participants