Terribly, weakly support use as a git dependency #456
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cript that builds dist/ if not already present. Won't work in Windows.
seanpdoyle
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Nov 23, 2021
Closes [hotwired#456][] The changes made in [hotwired#464][] resolved an issue in how `GET` requests merged query parameters into their `URL` instance. Unfortunately, the changes were too loosely scoped, and they affected _all_ `GET` requests, including those initiated by `<a>` elements with `[href]` attributes that declare search parameters directly (that *do not* need to be "merged" into anything). The [change][] made in [hotwired#464][] to conditionally include the `FetchRequest.body` based on the request's verb enables the changes made in this commit. In response to that change, this commit moves the `mergeFormDataEntries()` function out of the `FetchMethod` module and into the `FormSubmission` class. Since merging _from_ `FormData` instances _into_ a `URL` is a concern of `GET`-powered Form Submissions, moving the function out of the `FetchRequest` further simplifies that class. Move the `mergeFormDataEntries()` to the `FormSubmission` constructor to re-write its `location` property's [searchParams][] prior to constructing the instance's `FetchRequest`. In addition to that change, this commit removes the `FetchRequestBody` union type that combined `URLSearchParams` and `FormData`. In its place, change the `FetchRequest` to only accept `FormData` instances, since any `URLSearchParams` management will occur outside that class. [hotwired#456]: hotwired#465 [hotwired#464]: hotwired#464 [change]: hotwired@6906e57#diff-68b647dc2963716dc27c070f261d0b942ee9c00be7c4149ecb3a5acd94842d40R119 [searchParams]: https://developer.mozilla.org/en-US/docs/Web/API/URL/searchParams
seanpdoyle
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Nov 23, 2021
Closes [hotwired#456][] The changes made in [hotwired#464][] resolved an issue in how `GET` requests merged query parameters into their `URL` instance. Unfortunately, the changes were too loosely scoped, and they affected _all_ `GET` requests, including those initiated by `<a>` elements with `[href]` attributes that declare search parameters directly (that *do not* need to be "merged" into anything). The [change][] made in [hotwired#464][] to conditionally include the `FetchRequest.body` based on the request's verb enables the changes made in this commit. In response to that change, this commit moves the `mergeFormDataEntries()` function out of the `FetchMethod` module and into the `FormSubmission` class. Since merging _from_ `FormData` instances _into_ a `URL` is a concern of `GET`-powered Form Submissions, moving the function out of the `FetchRequest` further simplifies that class. Move the `mergeFormDataEntries()` to the `FormSubmission` constructor to re-write its `location` property's [searchParams][] prior to constructing the instance's `FetchRequest`. In addition to that change, this commit removes the `FetchRequestBody` union type that combined `URLSearchParams` and `FormData`. In its place, change the `FetchRequest` to only accept `FormData` instances, since any `URLSearchParams` management will occur outside that class. [hotwired#456]: hotwired#465 [hotwired#464]: hotwired#464 [change]: hotwired@6906e57#diff-68b647dc2963716dc27c070f261d0b942ee9c00be7c4149ecb3a5acd94842d40R119 [searchParams]: https://developer.mozilla.org/en-US/docs/Web/API/URL/searchParams
seanpdoyle
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Nov 23, 2021
Closes [hotwired#456][] The changes made in [hotwired#464][] resolved an issue in how `GET` requests merged query parameters into their `URL` instance. Unfortunately, the changes were too loosely scoped, and they affected _all_ `GET` requests, including those initiated by `<a>` elements with `[href]` attributes that declare search parameters directly (that *do not* need to be "merged" into anything). The [change][] made in [hotwired#464][] to conditionally include the `FetchRequest.body` based on the request's verb enables the changes made in this commit. In response to that change, this commit moves the `mergeFormDataEntries()` function out of the `FetchMethod` module and into the `FormSubmission` class. Since merging _from_ `FormData` instances _into_ a `URL` is a concern of `GET`-powered Form Submissions, moving the function out of the `FetchRequest` further simplifies that class. Move the `mergeFormDataEntries()` to the `FormSubmission` constructor to re-write its `location` property's [searchParams][] prior to constructing the instance's `FetchRequest`. [hotwired#456]: hotwired#465 [hotwired#464]: hotwired#464 [change]: hotwired@6906e57#diff-68b647dc2963716dc27c070f261d0b942ee9c00be7c4149ecb3a5acd94842d40R119 [searchParams]: https://developer.mozilla.org/en-US/docs/Web/API/URL/searchParams
dhh
pushed a commit
that referenced
this pull request
Nov 23, 2021
* Restore searchParams to Visit and Frame Navigation Closes [#456][] The changes made in [#464][] resolved an issue in how `GET` requests merged query parameters into their `URL` instance. Unfortunately, the changes were too loosely scoped, and they affected _all_ `GET` requests, including those initiated by `<a>` elements with `[href]` attributes that declare search parameters directly (that *do not* need to be "merged" into anything). The [change][] made in [#464][] to conditionally include the `FetchRequest.body` based on the request's verb enables the changes made in this commit. In response to that change, this commit moves the `mergeFormDataEntries()` function out of the `FetchMethod` module and into the `FormSubmission` class. Since merging _from_ `FormData` instances _into_ a `URL` is a concern of `GET`-powered Form Submissions, moving the function out of the `FetchRequest` further simplifies that class. Move the `mergeFormDataEntries()` to the `FormSubmission` constructor to re-write its `location` property's [searchParams][] prior to constructing the instance's `FetchRequest`. [#456]: #465 [#464]: #464 [change]: 6906e57#diff-68b647dc2963716dc27c070f261d0b942ee9c00be7c4149ecb3a5acd94842d40R119 [searchParams]: https://developer.mozilla.org/en-US/docs/Web/API/URL/searchParams * Attempt to fix flaky Scroll Restoration test There is a common test failure in CI: ``` × firefox on linux 5.11.0-1020-azure - ScrollRestorationTests - landing on an anchor (0.02s) 444 TypeError: Cannot read property 'slice' of null 445 at RemoteChannel.read @ src/tests/helpers/remote_channel.ts:14:44 ``` This commit addresses it in two ways. First, ensures that the `scroll_restoration.html` page loads the `src/tests/fixtures/test.js` script to wire-up the remote channels (like `window.eventLogs` and `window.mutationLogs`). Next, it modifies the test itself to await the `this.nextBody` hook, to ensure that the page load completes before checking the scroll.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just a proof of concept to share in case anyone else wants to package from a specific git version without having to republish the package, vendor it, or proxy it. Not to be recommended, or merged. 😂
Add as a git dependency. Be sure to use the full git URL or else Yarn will install using a tarball and bypass the prepare script.
Now the
preparescript is triggered before install, including devDependencies, so we can use it to build IF we detect we're installing from git vs an npm package (based on the presence of thedist/dir). Hence, won't work on Windows because this assumes having a shell available; would have to wrap that up in a prepare.js instead.Also note that Yarn has a history of funky bugs with concurrent invocations like this, so you may need to pass
--network-concurrency 1to youryarn installin CI and Dockerfiles and such.Worse, Yarn and npm have diverged significantly with which scripts run and when, whether prepack or prepare. The whole thing is a regrettable morass. I apologize for sharing this, but figure someone will duck-duck-go for "yarn git dependency build" or some nonsense and need somewhere puzzling to land.
💀