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

Complete PR #980 #1546

Merged
merged 169 commits into from
Aug 30, 2017
Merged

Complete PR #980 #1546

merged 169 commits into from
Aug 30, 2017

Conversation

pierreneter
Copy link
Member

Update webpack , antwar and dependencies.
Complete PR #980 (branch feat-webpack2).
Fix issues:

Achim Krüger and others added 30 commits June 28, 2017 23:07
Otherwise it's not possible to resolve imports in the entry file from other .ts or .tsx files
The sample output that was previously shown for `--profile` will not
appear unless `--progress` is also used. This change spells this out,
and also describes the per-module timings that `--profile`
produces by itself.
Solves the following error: `configuration.output.path: The provided 
value "dist" is not an absolute path!`. Now it also uses the same output 
path as the "Getting started"  >  "Using a Configuration" guide.
To make it easier to discover use case reported by webpack/webpack#4665 
from the documentation, as the existing array example only convey/imply 
that only modules are supported by this construct.
Fix definition of `webpack -p` command line option expansion, at the 
top of this page its missing the needed single quotes which are present 
down below.
We can choose to revive these later if they are requested by a significant
amount of people. Also, especially the content in compatibility.md, is covered
in other places (specifically in the `module-***.md` pages in `/api`).
This isn't really as much of an instructional as it is just information about
different bundling and build systems.
…1338)

Merge Code Splitting guides by removing all caching discussion 
and other extraneous content. Focuses on code splitting approaches 
instead. Move certain `import()` documentation to the Module API 
page. Use links to prevent content duplication. Replace lazy-load-react 
with a short, framework-agnostic lazy-loading guide.
Fix a tiny typo
Make some tweaks and and add a section on YAML frontmatter
and the available options.
Add documentation on passing a function for `target`. Also 
include some minor tweaks and formatting improvements.
This tip is no longer necessary as webpack/webpack#4771 was
fixed.
Unnecessary lingering comma @ the end of the exclude line
Appears to have been a clip-n-paste error from output.hashFunction's 
description.
Regularized capitalization/hyphenation of terms, clarified some sentence structure, and amended punctuation.
* Updated getting-started.md

Small changes to conclusion for better flow into next guide.

* fix: Textual changes to getting-started.md
Rewrite of this guide as a logical follow up of the `Asset Management` 
guide. Manifest section will be expanded on later.
evilebottnawi and others added 10 commits August 23, 2017 17:17
Add loader chaining and `resolveLoader` examples.
Removed the example of a Webpack setup for building to both 
electron and web because it uses an old plugin for electron-renderer 
support even though Webpack has native support for target.

Added example boilerplate which contains good examples of 
configuration files for both electron-main and electron-renderer.
Elaborate on the `devtool` options and explain in more detail how
each one affects the output source mapping and compilation speed.

fixes #273
fixes webpack/webpack#2725
fixes webpack/webpack#4936
fixes webpack/webpack#2766
fixes webpack/webpack#2145
fixes webpack/webpack#1689
Add an explanation of how to load polyfills on demand section in
the shimming guide.

Resolves #938
code = code.replace(/\[([^\[\]]+?)\]\((.+?)\)/g, match => {
match = /\[([^\[\]]+?)\]\((.+?)\)/.exec(match);
code = code.replace(/\[([^[\]]+?)\]\((.+?)\)/g, match => {
match = /\[([^[\]]+?)\]\((.+?)\)/.exec(match);
Copy link
Member Author

Choose a reason for hiding this comment

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

yarn run lint:js
$ eslint . --ext .js --ext .jsx

C:\Users\Pierre\Github\webpack.js.org\src\utilities\markdown.js
  43:33  error  Unnecessary escape character: \[  no-useless-escape
  44:23  error  Unnecessary escape character: \[  no-useless-escape

✖ 2 problems (2 errors, 0 warnings)

So, I removed that \.

@pierreneter
Copy link
Member Author

Hi @skipjack I have fixed #1085 with this branch.
The initial description for this PR is incomplete.
So I will summarize the content with this comment.

This PR will:

Complete:

Solve:

@skipjack
Copy link
Collaborator

skipjack commented Aug 26, 2017

@pierreneter thank you for digging into this. Can you be a bit more specific about how you fixed these issues? #980 was pretty much ready to go aside from the section.all() data. Did you upgrade antwar to fix that? I get that the other updates are just to bring #980 (feat-webpack2) up to date with the recent restructuring from #1490. It just makes it hard to tell what you changed to fix the issues there.

@pierreneter
Copy link
Member Author

Sure. Let me explain.
#980 is almost complete, and I'm just fixing some of the issues that it encounters.

  • Route: It has problems with recursive content filtering, make the article list of a section incorrect and display this list wrong in the sidebar:

captursssssssssssssssssssse

That's the recursion problem and the way to configure the paths in antwar.config.js

The origin code:

'/': {
      content: () => (
        require.context(
          './loaders/page-loader!./content',
          true,
          /^\.\/.*\.md$/
        )
      )
...
	paths: {
        'get-started': {
          redirects: {
            '': '/guides/getting-started',
            'install-webpack': '/guides/installation',
            'why-webpack': '/guides/why-webpack',
          }
        },
        guides: {
          redirects: {
            'code-splitting-import': '/guides/code-splitting-async',
            'code-splitting-require': '/guides/code-splitting-async/#require-ensure-',
            'why-webpack': '/guides/comparison',
            'production-build': '/guides/production'
          }
        },
        configuration: {
          redirects: {
            'external-configs': 'javascript-alternatives'
          }
        },
    ...
	}
}

The settings above make all posts have the same section.

My code makes configuring the path specified and does not have this problem anymore:

 paths: {
    '/': {
    ...
-       content: () => require.context('./src/loaders/page-loader!./src/content', true, /^\.\/.*\.md$/), //This causes an error when recursively
+       content: () => require.context('./src/loaders/page-loader!./src/content', false, /^\.\/.*\.md$/),
    ...
    },
    ...
    concepts: { // Move it out of the `paths` of '/'
        title: "Comcepts",
        content: () => require.context('./src/loaders/page-loader!./src/content/concepts', false, /^\.\/.*\.md$/),
        url: ({ sectionName, fileName }) => `/${sectionName}/${fileName}/`,
        transform: (pages) => {
            return _.sortBy(pages, (page) => page.file.sort)
        },
        layout: () => require('./src/components/Page/Page.jsx').default
    },
    ...
}

captssssssssssssssssssssssssssssssure

I added this feature with add line:

result.sort = result.attributes.sort || null;

into ./src/loaders/page-loader.js (https://github.com/pierreneter-repositories/webpack.js.org/blob/feat-webpack2/src/loaders/page-loader.js#L16)

And add it to new structure of antwar.config.js:


transform: (pages) => {
    return _.sortBy(pages, (page) => page.file.sort)
},

The origin code (https://github.com/webpack/webpack.js.org/blob/feat-webpack2/components/sidebar/sidebar.jsx#L31):

<SidebarItem
    url={ `/${sectionName}/` }
    title="Introduction"
    currentPage= { currentPage } />

{
pages.map(({ url, title, anchors }, i) =>
  <SidebarItem
    key={ `sidebar-item-${i}` }
    index={i}
    url={ `${url}` }
    title={ title }
    anchors={ anchors }
    currentPage= { currentPage }
    onToggle={ this._recalculate.bind(this) } />
)
}

My code (https://github.com/pierreneter-repositories/webpack.js.org/blob/feat-webpack2/src/components/Sidebar/Sidebar.jsx#L33):

<SidebarItem
    url={ `/${sectionName}/` }
    title="Introduction"
    currentPage= { currentPage }
+   anchors= { anchors } />

{
pages.map(({ url, title, anchors }, i) =>
  <SidebarItem
    key={ `sidebar-item-${i}` }
    index={i}
    url={ `${url}` }
    title={ title }
    anchors={ anchors }
    currentPage= { currentPage }
    onToggle={ this._recalculate.bind(this) } />
)
}

This anchors from: https://github.com/pierreneter-repositories/webpack.js.org/blob/feat-webpack2/src/components/Sidebar/Sidebar.jsx#L13

let { sectionName, pages, currentPage, anchors } = this.props;

Added from Page.jsx (https://github.com/pierreneter-repositories/webpack.js.org/blob/feat-webpack2/src/components/Page/Page.jsx#L45):

<Interactive
        id="src/components/Sidebar/Sidebar.jsx"
        component={ Sidebar }
        sectionName={ section.name }
        pages={ section.pages().map(({
          file: {
            attributes: {
              anchors,
              title
            }
          },
          url
        }) => ({ url, title, anchors })) }
        currentPage={ url.replace('/index', '') }
+       anchors={ page.file.attributes.anchors } />

So:

capturssssssssssssssssssssssssssssssssssse

  • Mobile Sidebar:

captursssssssssssssssssssssse

It errors because the "Retrieve section data" at Site.jsx (https://github.com/webpack/webpack.js.org/blob/feat-webpack2/components/site/site.jsx#L28) is out of date compared to the latest version of antwar:

The origin code:

let sections = section.all()
.map(({ title, url, pages }) => ({
  title,
  url,
  pages: pages.map(({ title, url }) => ({
    title: title || url, // XXX: Title shouldn't be coming in as undefined
    url
  }))
}));

My fix (https://github.com/pierreneter-repositories/webpack.js.org/blob/feat-webpack2/src/components/Site/Site.jsx#L27):

let sections = section.all().filter(section => section.path.hideInSidebar !== true)
.map((section) => {
  let _section = {
    title: section.path.title,
    url: section.url,
    pages: section.pages.map(page => {
      let _page = {
        title: page.file.title,
        url: page.url
      };
      return _page;
    })
  };
  return _section;
});

Why need section.path.hideInSidebar. Because of some section don't need show in sidebar, it's just a redirects, we don't need show it (there are just a few cases that we need), like (we can config at antwar.config.js):

pluginsapi: {
	title: 'API Plugin',
	redirects: {
		'': '/api/plugins',
		'compiler': '/api/plugins/compiler',
		'template': '/api/plugins/template'
	},
	hideSidebar: true
},

Mobile Sidebar fixed:

captssssssssssssssssssssssssure

  • And some other bug fixes in the process of merging with the master branch.

All done.

@pierreneter
Copy link
Member Author

Oops, it's hideInSidebar but I added hideSidebar: true at antwar.config.js. Fixed.
It work:

captssssssssssssssssssure

@pierreneter
Copy link
Member Author

nav
nav2

This is just an issue of #980.

Fixed with commit above.

@pierreneter
Copy link
Member Author

@skipjack
Copy link
Collaborator

@pierreneter ok I think I followed most of that, thanks for the detailed explanation. I think I'll merge this into feat-webpack2 ( #980 ) which should then yield a much more concise diff (Files Changed) that we can carefully review/test. Thanks again for your work on this and patience with the delayed reviews 👍 .

@skipjack skipjack merged commit b748ddc into webpack:feat-webpack2 Aug 30, 2017
@skipjack
Copy link
Collaborator

@pierreneter ok rolled everything up into one commit so we can easily roll that branch back if necessary. I'm going to check the diff there carefully now and test locally. If we can get #980 merged this week that would be fantastic 🎉 🎉

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.