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

Netlify-based redirect rules #38

Merged
merged 6 commits into from
Mar 30, 2022
Merged

Netlify-based redirect rules #38

merged 6 commits into from
Mar 30, 2022

Conversation

sdepold
Copy link
Member

@sdepold sdepold commented Mar 27, 2022

Supersedes #16. Can be tested here: https://capable-youtiao-134aad.netlify.app/

The problem is however, that the urls don't match:

I guess we could somehow walk through the file system and generate the redirects file automatically but is that worth it and do we really care enough to try?

Please note that the PR is currently only half working. /manual should be replaced with something else (as indicated above).

@netlify
Copy link

netlify bot commented Mar 27, 2022

Deploy Preview for capable-youtiao-134aad ready!

Name Link
🔨 Latest commit b035521
🔍 Latest deploy log https://app.netlify.com/sites/capable-youtiao-134aad/deploys/6241fb1c4d6ebd000889c3df
😎 Deploy Preview https://deploy-preview-38--capable-youtiao-134aad.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sdepold sdepold requested a review from ephys March 27, 2022 15:59
@sdepold
Copy link
Member Author

sdepold commented Mar 27, 2022

A push to this PR / branch should theoretically deploy the netlify page

@ephys
Copy link
Member

ephys commented Mar 27, 2022

do we really care enough to try?

It's pretty important as current search results will break if we don't
Shouldn't take too long, I already have both lists, I can look into it

The preview thingy doesn't seem to be working

@WikiRik
Copy link
Member

WikiRik commented Mar 27, 2022

Not sure if this is the best issue/PR to mention this but do we really need Netlify? Does GitHub Pages not satisfy our needs?

@ephys
Copy link
Member

ephys commented Mar 28, 2022

The reason behind the host change is that github doesn't provide a way to set redirection rules. We tried doing them on the front-end instead but docusaurus's plugin doesn't support redirecting from .html extensions.

(#16 (comment))

Netlify has an open source plan so it should be free for us maybe hopefully. The PR preview is also a nice bonus

@ephys
Copy link
Member

ephys commented Mar 28, 2022

Proposition for the redirection file:

/main/* /v6/:splat 302
/master/* /v6/:splat 302

/v6/identifiers.html    /api/v6/identifiers.html 302
/v6/class/*             /api/v6/class/:splat 302
/v6/variable/*          /api/v6/variable/:splat 302

/v6 /docs/v6/intro 302
/v6/manual/advanced-many-to-many.html           /docs/v6/advanced-association-concepts/advanced-many-to-many 302
/v6/manual/association-scopes.html              /docs/v6/advanced-association-concepts/association-scopes 302
/v6/manual/creating-with-associations.html      /docs/v6/advanced-association-concepts/creating-with-associations 302
/v6/manual/eager-loading.html                   /docs/v6/advanced-association-concepts/eager-loading 302
/v6/manual/polymorphic-associations.html        /docs/v6/advanced-association-concepts/polymorphic-associations 302
/v6/manual/assocs.html                          /docs/v6/core-concepts/assocs 302
/v6/manual/getters-setters-virtuals.html        /docs/v6/core-concepts/getters-setters-virtuals 302
/v6/manual/model-basics.html                    /docs/v6/core-concepts/model-basics 302
/v6/manual/model-instances.html                 /docs/v6/core-concepts/model-instances 302
/v6/manual/model-querying-basics.html           /docs/v6/core-concepts/model-querying-basics 302
/v6/manual/model-querying-finders.html          /docs/v6/core-concepts/model-querying-finders 302
/v6/manual/paranoid.html                        /docs/v6/core-concepts/paranoid 302
/v6/manual/raw-queries.html                     /docs/v6/core-concepts/raw-queries 302
/v6/manual/validations-and-constraints.html     /docs/v6/core-concepts/validations-and-constraints 302
/v6/manual/associations.html                    /docs/v6/moved/associations 302
/v6/manual/data-types.html                      /docs/v6/moved/data-types 302
/v6/manual/models-definition.html               /docs/v6/moved/models-definition 302
/v6/manual/models-usage.html                    /docs/v6/moved/models-usage 302
/v6/manual/querying.html                        /docs/v6/moved/querying 302
/v6/manual/aws-lambda.html                      /docs/v6/other-topics/aws-lambda 302
/v6/manual/connection-pool.html                 /docs/v6/other-topics/connection-pool 302
/v6/manual/constraints-and-circularities.html   /docs/v6/other-topics/constraints-and-circularities 302
/v6/manual/dialect-specific-things.html         /docs/v6/other-topics/dialect-specific-things 302
/v6/manual/extending-data-types.html            /docs/v6/other-topics/extending-data-types 302
/v6/manual/hooks.html                           /docs/v6/other-topics/hooks 302
/v6/manual/indexes.html                         /docs/v6/other-topics/indexes 302
/v6/manual/legacy.html                          /docs/v6/other-topics/legacy 302
/v6/manual/legal.html                           /docs/v6/other-topics/legal 302
/v6/manual/migrations.html                      /docs/v6/other-topics/migrations 302
/v6/manual/naming-strategies.html               /docs/v6/other-topics/naming-strategies 302
/v6/manual/optimistic-locking.html              /docs/v6/other-topics/optimistic-locking 302
/v6/manual/other-data-types.html                /docs/v6/other-topics/other-data-types 302
/v6/manual/query-interface.html                 /docs/v6/other-topics/query-interface 302
/v6/manual/read-replication.html                /docs/v6/other-topics/read-replication 302
/v6/manual/resources.html                       /docs/v6/other-topics/resources 302
/v6/manual/scopes.html                          /docs/v6/other-topics/scopes 302
/v6/manual/sub-queries.html                     /docs/v6/other-topics/sub-queries 302
/v6/manual/transactions.html                    /docs/v6/other-topics/transactions 302
/v6/manual/typescript.html                      /docs/v6/other-topics/typescript 302
/v6/manual/upgrade-to-v6.html                   /docs/v6/other-topics/upgrade-to-v6 302

This should cover everything but I'll still run a test on it once it's live in preview

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

I just updated the file which should eventually lead to a deployment of the website. Do I understand it right, that we are catering for a https://sequelize.org/main/manual/dialect-specific-things.html

via

https://sequelize.org/main/manual/dialect-specific-things.html
--> https://sequelize.org/v6/manual/dialect-specific-things.html
--> https://sequelize.org/docs/v6/other-topics/dialect-specific-things

So a 3 step redirect?

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

Also, once this is proven to work, I would

a) swap the 302 to 301
b) replace the gh-pages branch update with a direct upload to netlify using their CLI

@ephys
Copy link
Member

ephys commented Mar 28, 2022

So a 3 step redirect?

Yup!
It's going to be slightly slower but it's not going to matter much once browsers/search engines update their redirection cache

Actually right now it's 3 redirections:

sequelize.org/main/manual/dialect-specific-things.html
--> sequelize.org/v6/manual/dialect-specific-things.html
--> sequelize.org/docs/v6/other-topics/dialect-specific-things
--> sequelize.org/docs/v6/other-topics/dialect-specific-things/

Because we have trailing slashes enabled. We should either disable trailing slashes or add them to our redirection file.

Just tested (not everything, just this URL) and it looks good through

https://capable-youtiao-134aad.netlify.app/main/manual/dialect-specific-things.html

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

super nice :) I'm fine with trailing slash or without :D I don't really care

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

Actually. It seems that the browser side url resolution is not using a trailing slash hence it seems that both

https://capable-youtiao-134aad.netlify.app/docs/v6/core-concepts/getters-setters-virtuals
and
https://capable-youtiao-134aad.netlify.app/docs/v6/core-concepts/getters-setters-virtuals/

are valid depending on how you ended up on that page.

In that case I would probably vote for dropping the trailing slash

@ephys
Copy link
Member

ephys commented Mar 28, 2022

If the site is fully static, it's probably netlify that is adding the trailing slash

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

ah you have found it already. :D

@WikiRik
Copy link
Member

WikiRik commented Mar 28, 2022

The reason behind the host change is that github doesn't provide a way to set redirection rules. We tried doing them on the front-end instead but docusaurus's plugin doesn't support redirecting from .html extensions.

Ah, I missed that comment. Thanks!

@sdepold
Copy link
Member Author

sdepold commented Mar 28, 2022

I just added trailing slashes to the Docusaurus config

@sdepold sdepold enabled auto-merge (squash) March 28, 2022 18:14
@sdepold
Copy link
Member Author

sdepold commented Mar 30, 2022

As discussed on slack, I'm
Merging this now and we'll do a final review once everything is complete

@sdepold sdepold disabled auto-merge March 30, 2022 07:42
@sdepold sdepold merged commit 2dfbb9b into main Mar 30, 2022
@sdepold sdepold deleted the redirects branch March 30, 2022 07:42
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

Successfully merging this pull request may close these issues.

3 participants