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

fix(express): Add package.json for express #50554

Merged
merged 1 commit into from Jan 12, 2021

Conversation

quinnturner
Copy link
Contributor

@quinnturner quinnturner commented Jan 12, 2021

Adds a versioned dependency for @types/express for @types/express-serve-static-core ^4.17.18.
This fixes a breaking typing issue for @types/4.17.10 where it strictly requires @types/express-serve-static-core ^4.17.18.
Otherwise, all properties on Request are not defined.

Please review the existing discussion on this thread: #50390

We might be required to modify: https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/definitions-parser/allowedPackageJsonDependencies.txt Modification merged.

Please fill in this template.

@typescript-bot
Copy link
Contributor

typescript-bot commented Jan 12, 2021

@quinnturner Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ❌ Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 50554,
  "author": "quinnturner",
  "headCommitOid": "bb385e1224e07264d5aef8e63a8c077b79219298",
  "lastPushDate": "2021-01-12T19:10:53.000Z",
  "lastActivityDate": "2021-01-12T19:24:34.000Z",
  "maintainerBlessed": false,
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "express",
      "kind": "edit",
      "files": [
        {
          "path": "types/express/package.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "borisyankov",
        "CMUH",
        "puneetar",
        "dfrankland"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "stale",
      "reviewer": "andrewbranch",
      "date": "2021-01-12T17:31:12.000Z",
      "abbrOid": "f45cfc4"
    }
  ],
  "ciResult": "pass"
}

@typescript-bot
Copy link
Contributor

🔔 @borisyankov @CMUH @puneetar @dfrankland — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Jan 12, 2021
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Jan 12, 2021
@typescript-bot
Copy link
Contributor

@quinnturner The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

"private": true,
"dependencies": {
"@types/body-parser": "*",
"@types/express-serve-static-core": "^4.17.18",
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that as well, glad I got the second opinion. Thanks!

@typescript-bot
Copy link
Contributor

@quinnturner The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

quinnturner added a commit to quinnturner/DefinitelyTyped-tools that referenced this pull request Jan 12, 2021
Adds `@types/express-serve-static-core` to `allowedPackageJsonDependencies.txt`.

Required for DefinitelyTyped/DefinitelyTyped#50554.

`@types/express@4.17.10` requires `@types/express-serve-static-core@^4.17.18`.
All versions before `4.17.18` break typing.
@quinnturner
Copy link
Contributor Author

Submitted a request on allowedPackageJsonDependencies

microsoft/DefinitelyTyped-tools#193

Adds a versioned peer dependency for `@types/express` for `@types/express-serve-static-core` `^4.17.18`.
This fixes a breaking typing issue for `@types/4.17.10` where it strictly requires `@types/express-serve-static-core` `^4.17.18`.
Otherwise, all properties on `Request` are not defined.
@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Jan 12, 2021
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Jan 12, 2021
@danger-public
Copy link

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

express (unpkg)

was missing the following properties:

  1. Route

Generated by 🚫 dangerJS against bb385e1

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Jan 12, 2021
@typescript-bot
Copy link
Contributor

@andrewbranch Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

@andrewbranch
Copy link
Member

@AndrewLeedham, can you sign off?

Copy link
Contributor

@AndrewLeedham AndrewLeedham left a comment

Choose a reason for hiding this comment

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

Yes this looks good to me

@typescript-bot
Copy link
Contributor

👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings.

Let’s review the numbers, shall we?

Comparison details 📊
master #50554 diff
Batch compilation
Memory usage (MiB) 78.2 78.3 +0.1%
Type count 13165 13165 0%
Assignability cache size 2633 2633 0%
Language service
Samples taken 476 476 0%
Identifiers in tests 476 476 0%
getCompletionsAtPosition
    Mean duration (ms) 389.4 396.6 +1.8%
    Mean CV 9.1% 9.2%
    Worst duration (ms) 881.4 860.2 -2.4%
    Worst identifier req req
getQuickInfoAtPosition
    Mean duration (ms) 410.7 417.8 +1.7%
    Mean CV 8.7% 8.7%
    Worst duration (ms) 836.4 867.7 +3.8%
    Worst identifier get use

It looks like nothing changed too much. I won’t post performance data again unless it gets worse.

@typescript-bot typescript-bot added the Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance. label Jan 12, 2021
@weswigham weswigham merged commit e89f7d9 into DefinitelyTyped:master Jan 12, 2021
@typescript-bot
Copy link
Contributor

I just published @types/express@4.17.11 to npm.

@quinnturner quinnturner deleted the fix/types-express branch January 12, 2021 21:47
@quinnturner
Copy link
Contributor Author

Tested, here's my current yarn.lock:

"@types/express@*", "@types/express@^4.17.11":
  version "4.17.11"
  dependencies:
    "@types/body-parser" "*"
    "@types/express-serve-static-core" "^4.17.18"
    "@types/qs" "*"
    "@types/serve-static" "*"

LGTM ✅

@elibarzilay elibarzilay moved this from Needs Maintainer Review to Recently Merged in New Pull Request Status Board Jan 14, 2021
@typescript-bot typescript-bot removed this from Recently Merged in New Pull Request Status Board Jan 14, 2021
DatSwissGuy added a commit to DatSwissGuy/pokemon-app that referenced this pull request Jan 19, 2021
DatSwissGuy added a commit to DatSwissGuy/pokemon-app that referenced this pull request Jan 19, 2021
DatSwissGuy added a commit to DatSwissGuy/pokemon-app that referenced this pull request Jan 22, 2021
* Add PWA support via CLI. Add some icons... (#39)

* Update dependencies (as well as Angular 11 migration) (#31)

* Update angular core

* Update angular cli

* Update angular universal

* Update the rest

* Update dependencies 2 (#34)

* Update Angular core

* Update Angular material

* Update depencies, npm audit

* Remove package-lock from .gitignore

* General code restyling (#33)

* Edit editor config (IDEA): space between brackets for imports, double quotes

* Remove [NBSP]

* Rename types to pokemon-types.ts including classes and references.

* Set typescript version between 4.0.0 and 4.1.0 in package.json

* Replace generics with types in  generation collection item model

* Restyle footer. Format code and remove unnecessary imports.

* Use concat instead of + in getPokemonSvgUrl function.

* Rename constants for better reading.

* Clickable generation card (#35)

* Initial components and logic. All clicks on generation cards lead to generation 1.

* Add cards to generation item / pokemon list. Remove underline from generation list.

* Add subtitles for card, add function for version_group formatting.

* Added new subtitle for pokemon list from generation. Changed homescreen Pokemon to list 251 entries.

* Change return type of formatVersionGroups function

* Add initial pwa setup via CLI and add some icons

* Check dependencies and merge and update where necessary

* Add more icons

* Rename app in manifest

* Change theme color in webmanifest to whitesmoke

* Install on other machine, update package.json

* Remove reducer folder and files. (#42)

* Simple refactoring 1 (#43)

* Rename a whole lot of files, add folder structure for interfaces. Some changes to theme color.

* Modify gamesForGeneration.ts to use numbers instead of a string.

* Change witch case in generations.component.ts to use numbers from the gamesForGeneration object. Move ngFor iterator to anchor tag in html template.

* Add Apple touch icon and rename short name in manifest.

* Add data for generations (first / last Pokemon, games, platform) (#48)

* Add pokemon numbers for each generation.

* New file for consolidated generation data

* Replace gamesForGeneration and pokemonNumberForGeneration with new data set (generationData.ts). Update switch case in generations.component.ts.

* Add working generation list and links (#44)

* Add routing and logic for generations. Replace dreamworld svg with official artwork.

* Remove padding from pokemon image in mat-card.

* Remove pokemon component (entry page) and replace it with a home component.

* Add the missing home component.

* Fix folder structure fore home route. Rename functions.

* Add subtitle with Generation X Pokémon for mat-card.

* Fix issues (duplicated property, remove unnecessary component) from merge conflict.

* Add new model for main region. Add main region to card subtitle.

* Run obligatory dependency security updates

* Add title to generation (list of pokemon) page.

* Add ngIf directive to check for generation content. Restyle component.

* Fix DefinitelyTyped/DefinitelyTyped#50554

* Fix issue with generation title card rendering. (#57)

* Add pokemon boxart (Gen 1 - 5) (#49)

* Add first batch of games and boxarts.

* Add another batch of boxart.

* Add more boxart images. Start with mat-card styling / layout for games

* Add more boxart images. Start with mat-card styling / layout for games.

* Add more box art images.

* Add generationData for card subtitle. Add missing Pokemon Yellow.

* Add more boxart up to generation 5. Add generation title cards. Update styling.

* Add alt attribute to images.

* Change some styling.

* Change sidenav behaviour to be opened in desktop mode. (#59)

* Rename generation component into pokemon-list. (#61)
DatSwissGuy added a commit to DatSwissGuy/pokemon-app that referenced this pull request Jan 29, 2021
* Add PWA support via CLI. Add some icons... (#39)

* Update dependencies (as well as Angular 11 migration) (#31)

* Update angular core

* Update angular cli

* Update angular universal

* Update the rest

* Update dependencies 2 (#34)

* Update Angular core

* Update Angular material

* Update depencies, npm audit

* Remove package-lock from .gitignore

* General code restyling (#33)

* Edit editor config (IDEA): space between brackets for imports, double quotes

* Remove [NBSP]

* Rename types to pokemon-types.ts including classes and references.

* Set typescript version between 4.0.0 and 4.1.0 in package.json

* Replace generics with types in  generation collection item model

* Restyle footer. Format code and remove unnecessary imports.

* Use concat instead of + in getPokemonSvgUrl function.

* Rename constants for better reading.

* Clickable generation card (#35)

* Initial components and logic. All clicks on generation cards lead to generation 1.

* Add cards to generation item / pokemon list. Remove underline from generation list.

* Add subtitles for card, add function for version_group formatting.

* Added new subtitle for pokemon list from generation. Changed homescreen Pokemon to list 251 entries.

* Change return type of formatVersionGroups function

* Add initial pwa setup via CLI and add some icons

* Check dependencies and merge and update where necessary

* Add more icons

* Rename app in manifest

* Change theme color in webmanifest to whitesmoke

* Install on other machine, update package.json

* Remove reducer folder and files. (#42)

* Simple refactoring 1 (#43)

* Rename a whole lot of files, add folder structure for interfaces. Some changes to theme color.

* Modify gamesForGeneration.ts to use numbers instead of a string.

* Change witch case in generations.component.ts to use numbers from the gamesForGeneration object. Move ngFor iterator to anchor tag in html template.

* Add Apple touch icon and rename short name in manifest.

* Add data for generations (first / last Pokemon, games, platform) (#48)

* Add pokemon numbers for each generation.

* New file for consolidated generation data

* Replace gamesForGeneration and pokemonNumberForGeneration with new data set (generationData.ts). Update switch case in generations.component.ts.

* Add working generation list and links (#44)

* Add routing and logic for generations. Replace dreamworld svg with official artwork.

* Remove padding from pokemon image in mat-card.

* Remove pokemon component (entry page) and replace it with a home component.

* Add the missing home component.

* Fix folder structure fore home route. Rename functions.

* Add subtitle with Generation X Pokémon for mat-card.

* Fix issues (duplicated property, remove unnecessary component) from merge conflict.

* Add new model for main region. Add main region to card subtitle.

* Run obligatory dependency security updates

* Add title to generation (list of pokemon) page.

* Add ngIf directive to check for generation content. Restyle component.

* Fix DefinitelyTyped/DefinitelyTyped#50554

* Add function to add conjunction word to list.

* Add function to convert word to uppercase.

* Use helper function in generation component.

* Add helper function to get artwork image from species. Remove old function in pokemon-list.component.ts
kaznovac pushed a commit to kaznovac/DefinitelyTyped that referenced this pull request Mar 2, 2021
Adds a versioned peer dependency for `@types/express` for `@types/express-serve-static-core` `^4.17.18`.
This fixes a breaking typing issue for `@types/4.17.10` where it strictly requires `@types/express-serve-static-core` `^4.17.18`.
Otherwise, all properties on `Request` are not defined.
@roikoren755 roikoren755 mentioned this pull request Aug 10, 2021
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical package Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants