Skip to content

Force Algolia search to redirect to window.origin URL - #3404

Open
renatodex wants to merge 3 commits into
reactjs:mainfrom
renatodex:renatodex/redir-algolia-search-to-origin-url
Open

Force Algolia search to redirect to window.origin URL#3404
renatodex wants to merge 3 commits into
reactjs:mainfrom
renatodex:renatodex/redir-algolia-search-to-origin-url

Conversation

@renatodex

@renatodex renatodex commented Nov 24, 2020

Copy link
Copy Markdown
Contributor

Why did you do it?

While publishing a PR, I've noticed the autocomplete search at the header doesn't work on Vercel
Apps or the Local Environment.
What happens is that Algolia results are always returning production
URLs, and thereat any search in Local or Review Apps end up redirecting the user to reactjs.org.
This is not the expected behavior while testing, since you can accidentally leave the Vercel App without noticing, introducing all
kinds of confusion.

What changed?

With this small change, we are always enforcing the origin URL to be
used as the host for the redirect.
This will work both for production and any other environment.

Also, in the process of running lint, I received this warning:

6:7  warning  'path' is assigned a value but never used. Allowed unused vars must match /^_/  no-unused-vars

So I've also removed this unused path variable from one of the gatsby-node.js files.

Reproduction of the Issue:

https://share.getcloudapp.com/RBu9NyZb

While publishing a PR, I've noticed that search doesn't work on Vercel
Apps. This happens because Algolia results always default to production
URLs. This is not the expected behavior while testing, since you can
accidentally left the Vercel App without noticing, introducing all
kinds of confusions.

With this small change, we are always enforcing the origin URL to be
used as the host for the redirect.
This will work both for production an any other environment.
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

Hi @renatodex!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

inputSelector: '#algolia-doc-search',
handleSelected: function(input, event, suggestion, datasetNumber, context) {
const suggestionUrl = new URL(suggestion.url)
window.location = `${window.origin}${suggestionUrl.pathname}`

@renatodex renatodex Nov 24, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All suggestion urls are coming with a slash from Algolia, that's why I haven't added slash between window.origin and suggestionUrl.pathname

More info about handleSelected behavior:
https://docsearch.algolia.com/docs/behavior/

@reactjs-bot

reactjs-bot commented Nov 24, 2020

Copy link
Copy Markdown

Deploy preview for reactjs ready!

Built without sensitive environment variables with commit 558693c

https://deploy-preview-3404--reactjs.netlify.app

@facebook-github-bot

Copy link
Copy Markdown
Collaborator

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

*/

const crypto = require('crypto');
const path = require('path');

@renatodex renatodex Nov 24, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was just an unused var, reported as a warning on the lint test, so I've decided to remove it.
This was the error:
6:7 warning 'path' is assigned a value but never used. Allowed unused vars must match /^_/ no-unused-vars

@alexkrolick

Copy link
Copy Markdown
Collaborator

This is great, this has been a problem for a long time!

datasetNumber,
context,
) {
const suggestionUrl = new URL(suggestion.url);

@alexkrolick alexkrolick Nov 24, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this get a corejs polyfill? The URL API isn't implemented under IE11. Not sure if that is supported still on this site or not.

@renatodex renatodex Nov 24, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thats a good point!
Can we find a confirmation of supported browsers? I would love to use a Polyfill, but don’t want to do that if IE11 compatibility is not required/expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants