Authoring Libraries #1532

marioacc · 2017-08-21T00:52:35Z

Authoring Libraries
A little bit of more explanation was added as well as a warning regarding to issue 4824

jsf-clabot · 2017-08-21T00:52:36Z

All committers have signed the CLA.

skipjack · 2017-08-21T23:04:09Z

@marioacc awesome thank you for jumping on this, I will review shortly. Can you sign the CLA? Once you've done it once, you should be all set for future contributions from the same GitHub account.

skipjack · 2017-08-21T23:52:55Z

@marioacc the first thing I'm noticing is the CI failure. This is due to simple markdown formatting issues:

> [email protected] lint:markdown /home/travis/build/webpack/webpack.js.org
> markdownlint --config ./.markdownlint.json *.md ./content/**/*.md

./content/guides/author-libraries.md: 31: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 38: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 94: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 96: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 111: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 221: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 269: MD031 Fenced code blocks should be surrounded by blank lines
./content/guides/author-libraries.md: 113: MD032 Lists should be surrounded by blank lines
./content/guides/author-libraries.md: 130: MD032 Lists should be surrounded by blank lines
./content/guides/author-libraries.md: 271: MD032 Lists should be surrounded by blank lines

If you could start by resolving those, that would be great.

marioacc · 2017-08-23T20:30:57Z

@skipjack I already have signed the CLA, so when I click on the Details link, it tells me the CLA is already signed.
I will solve the markdown issues.

skipjack · 2017-08-23T21:43:48Z

@marioacc the issue seems to be that one of your commits was authored with a different user (this can happen if you push from work for example). If you rebase on master with your main account, which we have to anyway to resolve the conflicts, it should fix the CLA issues.

TheDutchCoder

👍 Thanks so much for taking this on!

I have a few items I'd like you to resolve, most of it grammar or spelling actually, so they should be quick to fix!

TheDutchCoder · 2017-08-28T12:39:43Z

content/guides/author-libraries.md

 ---

-webpack is a tool which can be used to bundle application code and also to bundle library code. If you are the author of a JavaScript library and are looking to streamline your bundle strategy then this document will help you.
+webpack is a tool which can be used to bundle application code and also to bundle library code. If you are the author of a JavaScript library and are looking to streamline your bundle strategy then this document will help you with the differents webpack configurations to expose your libraries as you think convenient.


s/differents/different

g/as you think/as you think is

Perfect, will make the changes.

@marioacc I would actually probably simplify/rephrase this as:

Aside from applications, webpack can also be used to bundle JavaScript libraries. The following guide is meant for library authors looking to streamline their bundling strategy.

What do you think?

I like it. The simpler, the better.

TheDutchCoder · 2017-08-28T12:41:33Z

content/guides/author-libraries.md


 ```javascript
-import * as webpackNumbers from 'webpack-numbers';
-
+import * as webpackNumbers from 'webpack-numbers';//ES2015 module import


Nitpicking, but can you add aspace before the comments?
Also decide on whether or not to use a space after the //, as currently both seem to be used ;)

In the other guides the comments are used in the following way:
// Comment text
var someJSCode = {}
Do you mind if I set the comments format in that way?

Also decide on whether or not to use a space after the //, as currently both seem to be used ;)

@TheDutchCoder yeah we should probably try to standardize it by padding both sides.

Do you mind if I set the comments format in that way?

That sounds good to me, though I'd probably pad that = sign as well (e.g. var someJSCode = {}) 😜.

TheDutchCoder · 2017-08-28T12:42:04Z

content/guides/author-libraries.md

    ...
 </script>
 </html>
 ```

+The configurations also can expose the library in the next ways:


g/next/following

TheDutchCoder · 2017-08-28T12:42:31Z

content/guides/author-libraries.md

+The configurations also can expose the library in the next ways:
+
+- Property in the global object, for node.
+- Property in the this object.


Maybe use this in back ticks so it shows up as code.

TheDutchCoder · 2017-08-28T12:43:06Z

content/guides/author-libraries.md

 For full library configuration and code please refer to [webpack-library-example](https://github.com/kalcifer/webpack-library-example)


 ## Configure webpack

-Now the agenda is to bundle this library
+Now the agenda is to bundle this library achieving the next goals:


Not sure about the word "agenda" here, seems overly formal.
Maybe use "plan" or "idea"?

I think "plan" is way better.

TheDutchCoder · 2017-08-28T12:44:18Z

content/guides/author-libraries.md

+- Exposing the library as a variable called `webpackNumbers`.
+- Being able to access the library inside Node.js.s
+
+Also, the consumer will be able to access` the library the `next ways:


This sentence has a stray code block and a grammar issue.

How about: "Also, the consumer will be able to access the library in the following ways:"

TheDutchCoder · 2017-08-28T12:46:07Z

content/guides/author-libraries.md

@@ -166,11 +222,29 @@ module.exports = {
 };
 ```

+W>At the moment of webpack 3.5.5, using the next configuration is not working properly as stated in the [issue 4824](https://github.com/webpack/webpack/issues/4824):


This sentence doesn't flow nicely, how about:

"With webpack 3.5.5, using the following configuration doesn't work properly as stated in issue 4824:"

TheDutchCoder · 2017-08-28T12:47:36Z

content/guides/author-libraries.md


-To make your library available for reuse, add `library` property in webpack configuration.
+To make your library available for reuse, add `library` property inside `output` in webpack configuration.


g/library property/the library property
g/in webpack configuration/in the webpack configuration

TheDutchCoder · 2017-08-28T12:49:18Z

content/guides/author-libraries.md

@@ -185,7 +259,7 @@ module.exports = {
 };
 ```

-This makes your library bundle to be available as a global variable when imported. To make the library compatible with other environments, add `libraryTarget` property to the config.
+This makes your library bundle to be available as a global variable named `webpackNumbers` when imported. To make the library compatible with other environments, add `libraryTarget` property to the config. This will add the differents options about how the library can be exposed.


g/to be available/available
s/differents/different

TheDutchCoder · 2017-08-28T12:49:31Z

content/guides/author-libraries.md

    }
    ...
 };
 ```

-If `library` is set and `libraryTarget` is not, `libraryTarget` defaults to `var` as specified in the [output configuration documentation](/configuration/output). See [`output.libraryTarget`](/configuration/output#output-librarytarget) there for a detailed list of all available options.
+You can expose the library in the next ways:


g/next/following

skipjack · 2017-08-29T04:21:28Z

@marioacc thanks for fixing the markdown issues and the CLA thing too. If you could rebase once more on master to resolve the conflicts caused by the restructuring in #1490, that would save me time 🙏. Not a big deal if not, once you make the requested changes we should be able to get this merged 👍 .

marioacc · 2017-08-31T22:36:10Z

@skipjack Can you please take a look at the project, I am not sure If I did the rebase in a proper way.

skipjack · 2017-09-01T13:20:46Z

Ah, yeah something's off here. I'll pop over to you're repo and see if I can fix it. Typically rebasing shouldn't create an extra commit. When you rebase on master you'd typically have to git push --force origin [branch-name] to fix the branch after. It looks like you may have done the rebase correctly but then pulled and pushed instead of just force pushing.

Explanations rewritten to be more understandable. Warning for bug added.

skipjack · 2017-09-01T13:49:02Z

@marioacc the issue should be fixed now. All I did was...

Back up your current master branch to a backup branch.
Then git reset --hard on master to the most recent commit prior to your changes.
Then git cherry-pickd your most recent commits from backup.
Then git push -f origin master to update your master branch.

Please double check that everything is as it should be. Like I said, I still have a backup locally so we can restore anything else if necessary. I can't push it to your fork because GitHub only gives me access to your repository for the branch used for the PR.

marioacc · 2017-09-01T16:58:16Z

@skipjack Thank you very much, I am sorry for the complications my rebase caused.
I took a look over the author-libraries.md in src/content/guides and the old one in content/guides and they are equal.
I will delete the content folder to be in sync with the original webpack repo.

skipjack · 2017-09-03T00:23:36Z

@marioacc somehow things got a bit funky here again. I'll try to resolve and then review, please don't merge or push anything else until that's done.

Minor fixes... - Code formatting - Full diffs for most instances - Some grammar fixes

skipjack · 2017-09-03T20:44:20Z

@marioacc I reviewed and the content was great. Just added one commit to clean things up a bit before I merge.

@TheDutchCoder if you wanted to review again that would be great. I'm going to merge this once the build passes just for the sake of pushing things forward as I think it's in a pretty good state. However if you catch anything, one of us can always submit a follow up PR.

To test a build, plus the I believe the review was addressed.

skipjack · 2017-09-03T21:29:03Z

Ok ran the tests locally as we're still seeing network issues on travis that cause the lint:links / hyperlink script to timeout. Everything ran fine with the exception of webpack-contrib/install-webpack-plugin#118 which doesn't relate to this PR. Merging...

marioacc mentioned this pull request Aug 21, 2017

Guides - Review and Simplify #1258

Closed

skipjack added Guides Enhancement labels Aug 21, 2017

skipjack requested review from skipjack and TheDutchCoder August 21, 2017 23:04

TheDutchCoder previously requested changes Aug 28, 2017

View reviewed changes

marioacc and others added 5 commits September 1, 2017 09:36

Explanation extended and ref.json file added

257df4b

Project Structure added

a5f17b0

Explanations rewritten to be more understandable. Warning for bug added.

CI issues related with markdown fixed.

ca58445

Grammar and spelling enhancements.

d9d601d

Comments format normalized.

a23f4c1

skipjack force-pushed the master branch from 44137d2 to a23f4c1 Compare September 1, 2017 13:43

skipjack force-pushed the master branch from 087ce16 to a23f4c1 Compare September 3, 2017 00:24

docs(guides): pre-merge clean up of author-libraries

297ea3b

Minor fixes... - Code formatting - Full diffs for most instances - Some grammar fixes

skipjack approved these changes Sep 3, 2017

View reviewed changes

Merge branch 'master' into master

bfd232d

skipjack merged commit 1c341f7 into webpack:master Sep 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Authoring Libraries #1532

Authoring Libraries #1532

marioacc commented Aug 21, 2017 •

edited

Loading

jsf-clabot commented Aug 21, 2017 •

edited

Loading

skipjack commented Aug 21, 2017

skipjack commented Aug 21, 2017

marioacc commented Aug 23, 2017 •

edited

Loading

skipjack commented Aug 23, 2017

TheDutchCoder left a comment

TheDutchCoder Aug 28, 2017

marioacc Aug 28, 2017

skipjack Aug 29, 2017

marioacc Aug 31, 2017

TheDutchCoder Aug 28, 2017

marioacc Aug 29, 2017 •

edited

Loading

skipjack Aug 29, 2017

TheDutchCoder Aug 28, 2017

TheDutchCoder Aug 28, 2017

TheDutchCoder Aug 28, 2017

marioacc Aug 29, 2017 •

edited

Loading

TheDutchCoder Aug 28, 2017

TheDutchCoder Aug 28, 2017

TheDutchCoder Aug 28, 2017

TheDutchCoder Aug 28, 2017

TheDutchCoder Aug 28, 2017

skipjack commented Aug 29, 2017

marioacc commented Aug 31, 2017

skipjack commented Sep 1, 2017

skipjack commented Sep 1, 2017 •

edited

Loading

marioacc commented Sep 1, 2017

skipjack commented Sep 3, 2017

skipjack commented Sep 3, 2017

skipjack commented Sep 3, 2017


		To make your library available for reuse, add `library` property in webpack configuration.
		To make your library available for reuse, add `library` property inside `output` in webpack configuration.

Authoring Libraries #1532

Authoring Libraries #1532

Conversation

marioacc commented Aug 21, 2017 • edited Loading

jsf-clabot commented Aug 21, 2017 • edited Loading

skipjack commented Aug 21, 2017

skipjack commented Aug 21, 2017

marioacc commented Aug 23, 2017 • edited Loading

skipjack commented Aug 23, 2017

TheDutchCoder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioacc Aug 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioacc Aug 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skipjack commented Aug 29, 2017

marioacc commented Aug 31, 2017

skipjack commented Sep 1, 2017

skipjack commented Sep 1, 2017 • edited Loading

marioacc commented Sep 1, 2017

skipjack commented Sep 3, 2017

skipjack commented Sep 3, 2017

skipjack commented Sep 3, 2017

marioacc commented Aug 21, 2017 •

edited

Loading

jsf-clabot commented Aug 21, 2017 •

edited

Loading

marioacc commented Aug 23, 2017 •

edited

Loading

marioacc Aug 29, 2017 •

edited

Loading

marioacc Aug 29, 2017 •

edited

Loading

skipjack commented Sep 1, 2017 •

edited

Loading