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

[WIP] feat: add documentation links in schema #13822

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

snitin315
Copy link
Member

What kind of change does this PR introduce?
feature, add links to show in the error output.

Did you add tests for your changes?
WIP

Does this PR introduce a breaking change?
no

What needs to be documented once your changes are merged?
Nothing

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

@chenxsan Maybe we can add built-in redirects (for example https://webpack.js.org/webpack-options/output/chunk-load-timeout) and use them here, because options can be moved inside our site on other pages it requires update schema very often, using redirect we will solve it

@chenxsan
Copy link
Member

@alexander-akait With the current stacks of the webpack document site (A single page application hosted on github pages), we can't support server side redirects. However we can support client side redirection for urls of https://webpack.js.org/r/webpack/?option=output/chunkLoadTimeout pattern with just one intermediate page - r/webpack.mdx in this case. Pattern like https://webpack.js.org/webpack-options/output/chunk-load-timeout would require us to generate a lot of intermediate pages.

Do you think it's reasonable with a pattern like https://webpack.js.org/r/webpack/?option=output/chunkLoadTimeout? It's not the best option to me -- a server side redirect would be much better in my opinion, but it's the best trade-off we can have as far as I know.

Another option is to host the site on maybe vercel alike service which does support server side redirects.

@alexander-akait
Copy link
Member

@chenxsan I agree server side redirects are better, but maybe we can emulate right now redirects using <meta http-equiv="refresh" content="0; url=http://example.com/" /> and then discuss about migrate on vercel/etc

@chenxsan
Copy link
Member

@chenxsan I agree server side redirects are better, but maybe we can emulate right now redirects using <meta http-equiv="refresh" content="0; url=http://example.com/" /> and then discuss about migrate on vercel/etc

That would require us to generate a lot of intermediate pages as far as I know.

E.g., with a url like https://webpack.js.org/webpack-options/output/chunk-load-timeout, we need to generate a html file under dist/webpack-options/output/chunk-load-timeout/index.html with content like:

<meta http-equiv="refresh" content="0; url=https://webpack.js.org/configuration/output/#outputchunkloadtimeout" />

Here's an example we already have https://github.com/webpack/webpack.js.org/blob/gh-pages/loaders/file-loader/index.html which is created by RedirectWebpackPlugin. But I want to remind you that it won't work against urls with a hash like #amd, see webpack/webpack.js.org#5146 (comment).

The client side redirect might be the only feasible one besides the server side redirect at the moment.

@alexander-akait
Copy link
Member

@chenxsan Then let's do this for now and improve this in future

@webpack-bot
Copy link
Contributor

@snitin315 The most important CI builds failed. This way your PR can't be merged.

Please take a look at the CI results from azure (1 errors / 0 warnings) and appveyor (success) and fix these issues.

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.

None yet

4 participants