Plugin: Adding a build process #282

youknowriad · 2017-03-16T09:51:09Z

This includes:

Using yarn
Using babel ES-latest
Using Webpack
Using ESlint (extends Calypso's rules for now). Could we use prettier?
Using Sass and autoprefixer

This includes: - Using yarn - Using babel ES-latest - Using Webpack - Using ESlint (extends Calypso's rules for now) - Using Sass and auto-prefixer

mtias · 2017-03-16T11:11:14Z

plugin/webpack.config.js

+
+const config = module.exports = {
+ entry: {
+ app: './src/index.js'


I was thinking we should call this folder editor, and it would more closely represent what we may be bringing into core.

What about the distinction src/build? Do you think we should use editor as a parent directory or it's ok to keep build as is?

I may put the build in the root of the plugin.

aduth · 2017-03-16T12:29:55Z

For many of these items we should include a "Why?" and "Do we foresee this being a challenge being integrated into core?"

Yarn: Might be objectively good, but is another install when npm on its own works well enough
babel-latest: Probably fine on the specification line; may want to try to avoid unstable "staged" APIs (though object spread is awfully tempting). Concern here is education on new features.
Webpack: I see this being a simple enough transition from Browserify, but we should still decide what it offers that couldn't be achieved with existing tools?
ESLint: We may not need this in the plugin directory, per Add base ESLint and EditorConfig configurations #166 . There's an existing "WordPress" shared configuration that I've extended there with a few more specific (and to my knowledge non-conflicting) rules
prettier: I'd argue no, unless it can be configured per WordPress styles
Sass and autoprefixer: Already used in core, should be fine

aduth · 2017-03-16T12:31:17Z

plugin/.eslintrc.json

+ "node": true
+ },
+ "parserOptions": {
+ "ecmaVersion": 3


Likely ignored with babel-eslint parser override. Do we need to use babel-eslint or is the default parser sufficient?

From the docs:

You only need to use babel-eslint if you are using types (Flow) or experimental features not supported in ESLint itself yet

We may need it, if we decide to add object-spread, but we could drop it for now.

aduth · 2017-03-16T12:38:16Z

plugin/editor/assets/stylesheets/main.scss

+.gutenberg__editor {
+ max-width: 700px;
+ margin: 0 auto;
+ img {


aduth · 2017-03-16T12:38:32Z

plugin/package.json

+ "keywords": [
+ "WordPress",
+ "editor",
+ "prototype"


Not a prototype 😄

aduth · 2017-03-16T12:40:23Z

plugin/index.php

@@ -21,7 +21,7 @@ function gutenberg_menu() {

 function gutenberg_scripts_and_styles( $hook ) {
 if ( $hook === 'toplevel_page_gutenberg' ) {
- wp_enqueue_style( 'gutenberg_css', plugins_url( 'style.css', __FILE__ ) );


Curious if extract-text-webpack-plugin (what we'd eventually want to use to pull out CSS) can be integrated easily into a development configuration.

youknowriad · 2017-03-16T12:43:02Z

Yarn: Might be objectively good, but is another install when npm on its own works well enough

Yes, the advantages of yarn are locking properly and performance IMO.

may want to try to avoid unstable "staged" APIs (though object spread is awfully tempting.

There's no "staged" API on "babel-latest". I was also tempted to add "object spread" but I thought it could be done later if we have a lot of Object.assign :)

Prettier

I do agree, I think if it needs to be done, it should probably be a core first decision.

There's an existing "WordPress" shared configuration

I think the plugin directory should be self-sufficient (could be moved to its own repository), I don't mind duplicating the same .eslintrc.json from the root folder. I was not sure if the wordpress rules work well for ESnext, that's why I've used calypso's rules at first.

Consider the first commit of the PR as my own choices and I'm happy to update them to match the PR discussions :)

aduth · 2017-03-16T12:54:44Z

Yes, the advantages of yarn are locking properly and performance IMO.

My own opinion is that Yarn will follow the same story of HHVM vs. PHP, where the project motivates upstream improvements. PHP7 made HHVM irrelevant for many, and in the same way I hope npm will eventually adopt many of the ideas of Yarn. It's also one fewer prerequisites for getting started, which should always be a chief concern for helping contributors become involved.

There's no "staged" API on "babel-latest".

Sorry, I was unclear. I understand this and am fine with babel-latest.

I was also tempted to add "object spread" but I thought it could be done later if we have a lot of Object.assign :)

Yeah, would be wise to find ourselves needing it before preemptively deciding it'll be useful.

aduth · 2017-03-16T12:56:20Z

I don't mind duplicating the same .eslintrc.json from the root folder. I was not sure if the wordpress rules work well for ESnext, that's why I've used calypso's rules at first.

You raise a good point that many of those rules may not be applicable, or could be improved with ES2015+ specific additions. I'd still rather at least enumerate them explicitly than extend Calypso's ruleset (which includes a few style variations from WordPress' own).

youknowriad · 2017-03-16T13:08:53Z

PR updated.

Should we git ignore the build folder and rely on the GitHub releases? any decision here @aduth

aduth · 2017-03-16T13:23:50Z

Should we git ignore the build folder and rely on the GitHub releases? any decision here @aduth

My view is yes, to leave it out of the repo. While this will make the plugin unusable out of the box on master, it's more likely we'll want previews on tagged releases anyways. We always have the option to add it later if we decide it's not working well.

youknowriad · 2017-03-16T14:23:17Z

the build is ignored now :)

Can I have a 👍 to merge this or are there any other things we could discuss. BTW, we could always iterate on these later :)

aduth · 2017-03-16T14:35:54Z

plugin/index.php

@@ -21,7 +21,7 @@ function gutenberg_menu() {

 function gutenberg_scripts_and_styles( $hook ) {
 if ( $hook === 'toplevel_page_gutenberg' ) {
- wp_enqueue_style( 'gutenberg_css', plugins_url( 'style.css', __FILE__ ) );
+ wp_enqueue_script( 'gutenberg_js', plugins_url( 'build/app.js', __FILE__ ) );


This 404's because it's loading from the Gutenberg root. Should be plugin/build/app.js.

aduth

While my previous remark is true, it also speaks to issues of using repository directly as plugin with plugin code living in a subdirectory. I think we'll need to move plugin/index.php to the top-level directory, or at least symlink it. We should do that in a follow-up PR and merge this as is 👍

This includes: - Using babel ES-latest - Using Webpack - Using ESlint - Using Sass and auto-prefixer

Plugin: Adding a build process to the plugin

1640f74

This includes: - Using yarn - Using babel ES-latest - Using Webpack - Using ESlint (extends Calypso's rules for now) - Using Sass and auto-prefixer

youknowriad self-assigned this Mar 16, 2017

youknowriad requested review from mtias and aduth March 16, 2017 09:51

youknowriad changed the title ~~Plugin: Adding a build process to the plugin~~ Plugin: Adding a build process Mar 16, 2017

youknowriad requested a review from dmsnell March 16, 2017 11:00

mtias reviewed Mar 16, 2017

View reviewed changes

Plugin Build: Renaming src to editor

6474e1d

aduth reviewed Mar 16, 2017

View reviewed changes

youknowriad added 2 commits March 16, 2017 14:00

Plugin Build: Using WordPress Esling Config

36cadd1

Plugin Build: Removing Yarn

1784164

Plugin Build: git ignore the build folder

13f2269

aduth reviewed Mar 16, 2017

View reviewed changes

aduth approved these changes Mar 16, 2017

View reviewed changes

youknowriad merged commit 6e742ee into master Mar 16, 2017

youknowriad deleted the add/plugin/bootstrap-build-process branch March 16, 2017 14:42

nylen pushed a commit that referenced this pull request Mar 16, 2017

Plugin: Adding a build process (#282)

8f09591

This includes: - Using babel ES-latest - Using Webpack - Using ESlint - Using Sass and auto-prefixer

yassine4999 mentioned this pull request Apr 24, 2023

[Snyk] Security upgrade cosmiconfig from 7.1.0 to 8.0.0 yassine4999/gutenberg#10

Open

captainbarber99 mentioned this pull request Apr 24, 2023

[Snyk] Security upgrade cosmiconfig from 7.1.0 to 8.0.0 captainbarber99/gutenberg#10

Open

soloinovator mentioned this pull request Apr 24, 2023

[Snyk] Security upgrade cosmiconfig from 7.1.0 to 8.0.0 soloinovator/gutenberg#83

Open

npneeraj mentioned this pull request Apr 25, 2023

[Snyk] Security upgrade cosmiconfig from 7.1.0 to 8.0.0 npneeraj/gutenberg#14

Open

naiba4 mentioned this pull request Apr 25, 2023

[Snyk] Security upgrade cosmiconfig from 7.1.0 to 8.0.0 naiba4/gutenberg#8

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin: Adding a build process #282

Plugin: Adding a build process #282

youknowriad commented Mar 16, 2017 •

edited

Loading

mtias Mar 16, 2017

youknowriad Mar 16, 2017

mtias Mar 16, 2017

aduth commented Mar 16, 2017

aduth Mar 16, 2017

youknowriad Mar 16, 2017

aduth Mar 16, 2017

aduth Mar 16, 2017

aduth Mar 16, 2017

youknowriad commented Mar 16, 2017 •

edited

Loading

aduth commented Mar 16, 2017

aduth commented Mar 16, 2017

youknowriad commented Mar 16, 2017

aduth commented Mar 16, 2017

youknowriad commented Mar 16, 2017

aduth Mar 16, 2017

aduth left a comment

Plugin: Adding a build process #282

Plugin: Adding a build process #282

Conversation

youknowriad commented Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Mar 16, 2017

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

youknowriad commented Mar 16, 2017 • edited Loading

aduth commented Mar 16, 2017

aduth commented Mar 16, 2017

youknowriad commented Mar 16, 2017

aduth commented Mar 16, 2017

youknowriad commented Mar 16, 2017

Choose a reason for hiding this comment

aduth left a comment

Choose a reason for hiding this comment

youknowriad commented Mar 16, 2017 •

edited

Loading

youknowriad commented Mar 16, 2017 •

edited

Loading