Skip to content

feat(react-dev-utils): add redirectServedPathMiddleware and getPublicUrlOrPath #40982

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.

Sign up for GitHub

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

Jump to bottom
Merged
sandersn merged 1 commit into from
Feb 13, 2020
Merged

feat(react-dev-utils): add redirectServedPathMiddleware and getPublicUrlOrPath #40982

sandersn merged 1 commit into from
Feb 13, 2020

Conversation

@unrevised6419
Copy link

@unrevised6419 unrevised6419 commented Dec 11, 2019 •
edited
Loading

Related facebook/create-react-app/pull/7259
Related facebook/create-react-app/pull/8442

This was referenced Dec 11, 2019
Merged
Open
@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 12, 2019 •
edited
Loading

@iamandrewluca Thank you for submitting this PR!

🔔 @ark120202 - 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.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 12, 2019 •
edited
Loading

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

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

@typescript-bot typescript-bot added the Owner Approved A listed owner of this package signed off on the pull request. label Dec 12, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 12, 2019

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

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

@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 12, 2019

👋 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 #40982 diff
Batch compilation
Memory usage (MiB) 71.8 71.7 -0.2%
Type count 13877 13881 0%
Assignability cache size 4293 4293 0%
Language service
Samples taken 154 177 +15%
Identifiers in tests 154 177 +15%
getCompletionsAtPosition
    Mean duration (ms) 409.9 414.0 +1.0%
    Mean CV 9.5% 9.0%
    Worst duration (ms) 494.0 516.9 +4.6%
    Worst identifier checkRequiredFiles results
getQuickInfoAtPosition
    Mean duration (ms) 409.3 417.8 +2.1%
    Mean CV 9.3% 9.7% +3.7%
    Worst duration (ms) 536.1 582.7 +8.7%
    Worst identifier CLIEngine eslintFormatter

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

@typescript-bot typescript-bot added Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance. Merge:Express and removed The Travis CI build failed labels Dec 12, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 12, 2019

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@unrevised6419
Copy link
Author

unrevised6419 commented Dec 12, 2019

Let's wait for main feature to be merged!

@typescript-bot typescript-bot added Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. and removed Merge:Express labels Dec 20, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 20, 2019

@iamandrewluca Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

@typescript-bot typescript-bot added the Popular package This PR affects a popular package (as counted by NPM download counts). label Dec 31, 2019
@typescript-bot typescript-bot added The Travis CI build failed and removed Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. labels Feb 12, 2020
@typescript-bot
Copy link
Contributor

typescript-bot commented Feb 12, 2020

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

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

@unrevised6419 unrevised6419 marked this pull request as ready for review February 12, 2020 23:33
@unrevised6419 unrevised6419 changed the title feat(react-dev-utils): add redirectServedPathMiddleware middleware feat(react-dev-utils): add redirectServedPathMiddleware and getPublicUrlOrPath Feb 12, 2020
@sandersn sandersn merged commit 0a7ce6b into DefinitelyTyped:master Feb 13, 2020
@unrevised6419 unrevised6419 deleted the facebook/create-react-app/pull/7259 branch February 14, 2020 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ark120202 ark120202 ark120202 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Owner Approved A listed owner of this package signed off on the pull request. Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance. Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@unrevised6419 @typescript-bot @ark120202 @sandersn