Node module #706

javan · 2017-08-28T14:13:31Z

Moves the bulk of our webpack config into a node module instead of copying it all to apps on webpacker:install. There are only four files copied to config/webpack/ now:

1 config/webpack/environment.js for setting up base configuration:

const { environment } = require('webpacker')
module.exports = environment

2-4 config/webpack/{development,production,test}.js, which build off environment.js and are identical to start:

const environment = require('./environment')
module.exports = environment.toWebpackConfig()

TODO

Before merging

Relax dependency versions?
Update docs

After merging

Publish to npm
- $ npm version major
- $ npm publish --access public

Fixes #649
Closes #264

gauravtiwari · 2017-08-28T17:55:34Z

@javan Just briefly looked at your changes and it's looking really good 👍 💯

Happy to help with docs if you like or perhaps we can do it in separate PR i.e. move all README contents into docs/ folder or something. I can do that in parallel.

For node_modules, seems like someone already got https://www.npmjs.com/package/webpacker registered 😞

Mostly so we're not locked in to exporting a single value

…nfiguration

javan · 2017-08-29T20:32:25Z

@gauravtiwari, I added new documentation and touched up some of the existing bits in 34142b1. Let me know if I missed anything critical.

gauravtiwari · 2017-08-29T21:41:21Z

@javan Sure thing, reviewing now. The API looks really nice though 💯

We haven't published the package to npm yet right?

javan · 2017-08-29T21:44:14Z

Right. Not published yet. Will eventually be published as a scoped package: @rails/webpacker.

Side note: We should gut the README as soon as this is merged.

gauravtiwari · 2017-08-29T21:51:45Z

@javan Cool 👍

Yepp, will do. Thinking we create a docs/ folder and create .md for each topic. What do you think?

javan · 2017-08-29T22:12:20Z

We can discuss more in #645, but basically I think we should remove everything that's webpack-not-webpacker related.

javan · 2017-08-29T22:15:12Z

@gauravtiwari, can you please try this out in a real app?

You can do:

# Gemfile
gem 'webpacker', github: 'rails/webpacker', branch: 'node-module'

// package.json
"dependencies": {
  "webpacker": "rails/webpacker#node-module"
}

and then rails webpacker:install

gauravtiwari · 2017-08-29T22:17:49Z

@javan Yepp doing that now.

BTW, is code like this intentional i.e. if block and rest of the code, no space?

 const { extensions } = config
  if (!extensions.length) {
    throw new Error('You must configure at least one extension to compile in webpacker.yml')
  }
  return extensions.length === 1 ? `**/${extensions[0]}` : `**/*{${extensions.join(',')}}`

I corrected a few and if you don't mind I can push :)

javan · 2017-08-29T22:23:46Z

Hmm, not seeing that locally or on github

webpacker/package/environment.js

Lines 33 to 39 in c4bf1c2

    
           function getExtensionsGlob() { 
        
             const { extensions } = config 
        
             if (!extensions.length) { 
        
               throw new Error('You must configure at least one extension to compile in webpacker.yml') 
        
             } 
        
             return extensions.length === 1 ? `**/${extensions[0]}` : `**/*{${extensions.join(',')}}` 
        
           }

gauravtiwari · 2017-08-29T22:25:29Z

Oops sorry I meant lb before/after if block.

javan · 2017-08-29T22:27:41Z

I prefer the spacing as-is since it's a relatively short block of code. And the linter isn't complaining. 😬

gauravtiwari · 2017-08-29T22:45:25Z

package/index.js

+const { existsSync } = require('fs')
+
+function createEnvironment() {
+  const path = `./environments/${process.env.NODE_ENV}`


@javan Missing file extension here

Not needed. I removed them all throughout the codebase. Just like Ruby, Node's require doesn't need an extension.

but existsSync won't work, it requires a path

const path = resolve(__dirname, `./environments/${process.env.NODE_ENV}.js`)

https://nodejs.org/api/fs.html#fs_fs_existssync_path

const constructor = existsSync(path) ? require(path) : Environment

Will always evaluate to false and return default config

D'oh, right you are. Great catch! Fixed in df21fae

gauravtiwari · 2017-08-29T22:55:52Z

package.json

  "description": "Webpacker makes it easy to use the JavaScript preprocessor and bundler [Webpack](https://webpack.github.io) to manage application-like JavaScript in Rails. It coexists with the asset pipeline, as the purpose is only to use Webpack for app-like JavaScript, not images, css, or even JavaScript Sprinkles (that all continues to live in app/assets).",
-  "main": "index.js",
+  "main": "package/index.js",


Need to add files we are distributing from NPM so the package doesn't include ruby files.

"files": [ "package" ]

👍 e0ba006

gauravtiwari · 2017-08-30T08:36:58Z

Great, will take a final look now and we can merge

javan · 2017-08-30T12:12:43Z

README.md

+  test:  /.(ts|tsx)$/,
+  loader: 'ts-loader'
+})
+```


I removed this step because we're adding all loaders by default now, but didn't notice the tsx extension part. Maybe we should just add it to the loader?

webpacker/package/loaders/typescript.js

Lines 1 to 4 in 3cd2e49

module.exports = {

test: /\.ts$/,

loader: 'ts-loader'

}

@javan Oh yes, totally forgot. Yes, makes sense 👍

javan · 2017-08-30T12:15:11Z

README.md

-}
+// Update an option directly
+const vueLoader = environment.loaders.get('vue')
+vueLoader.test = /\.vue(\.erb)?$/


Should we make this the default test for the vue loader?

webpacker/package/loaders/vue.js

Line 7 in 3cd2e49

test: /\.vue$/,

Yeah we can do that but I guess then we would need to update all loaders to support .erb (if we want that out of the box)?

We already do for ~half of the loaders. Might as well for all?

Sounds good 👍 We also received this through many PR's and issues so makes sense 💯

Cool. Want to make that change (and also add tsx to the typescript loader)?

gauravtiwari · 2017-08-30T12:39:24Z

All done 👍 added an small change to dev server to add overlay for errors - this will basically show a screen with error instead of blank screen.

javan · 2017-08-30T12:44:09Z

README.md

@@ -90,7 +90,7 @@ in which case you may not even need the asset pipeline. This is mostly relevant

 ## Features

-* [Webpack 2](https://webpack.js.org/)
+* [Webpack 2 and 3](https://webpack.js.org/)


Maybe drop the 2? We technically don't support it now:

webpacker/package.json

Line 37 in b4ded38

"webpack": "^3.5.5",

Hmm yeah, although the config we have will work with both versions but makes sense to just say 3.

Also, we should add that module concatenation plugin? https://github.com/webpack/webpack/tree/master/examples/scope-hoisting#webpackconfigjs

Will push an update now 👍

The config may work, but you won't be able to install v2 if you have webpacker as a dependency in package.json

https://docs.npmjs.com/misc/scope#requiring-scoped-packages

…dule * 'node-module' of github.com:rails/webpacker: Fix require for scoped package

This was referenced Aug 28, 2017

[WIP] Move webpack config to node package #264

Closed

Creating a node module for webpacker... #649

Closed

javan force-pushed the node-module branch from 3edf034 to 34142b1 Compare August 29, 2017 20:26

javan added 6 commits August 29, 2017 16:28

Move base webpack config from app to new webpacker node module

05777ce

Update installer templates

e1b1d7e

Appease the linter

1474d39

Export an environment instance *and* the base class

eca739a

Mostly so we're not locked in to exporting a single value

Add environment.js, a base Environment instance, for adding global co…

012a951

…nfiguration

Document the new in-app webpack configuration files and API

34142b1

javan added 4 commits August 29, 2017 16:58

Prep for release

45dcb0e

Update to scoped package name

690974e

✂️

b8530ea

node_module → package

ce3d6c7

💅🏻 spaces and spacing

c4bf1c2

javan changed the title ~~WIP: Node module~~ Node module Aug 29, 2017

gauravtiwari reviewed Aug 29, 2017

View reviewed changes

javan added 2 commits August 29, 2017 20:27

Resolve full file path + name to fix existsSync check

df21fae

Specify files → omit the Ruby parts

e0ba006

Update old loaders in README

3cd2e49

gauravtiwari force-pushed the node-module branch from b7d8aea to 3cd2e49 Compare August 30, 2017 09:12

gauravtiwari approved these changes Aug 30, 2017

View reviewed changes

javan commented Aug 30, 2017

View reviewed changes

Remove loader step since it's present by default

892cc19

javan commented Aug 30, 2017

View reviewed changes

gauravtiwari added 2 commits August 30, 2017 13:29

Add erb support for all loaders

a6aad0d

Add overlay option for debugging

b4ded38

javan commented Aug 30, 2017

View reviewed changes

gauravtiwari and others added 5 commits August 30, 2017 13:51

Change to 3.x.x

d577ca2

Add ModuleConcatenation to production bundle

5e2767b

Fix require for scoped package

bf62258

https://docs.npmjs.com/misc/scope#requiring-scoped-packages

Add package scope

c0de660

Merge branch 'node-module' of github.com:rails/webpacker into node-mo…

2146503

…dule * 'node-module' of github.com:rails/webpacker: Fix require for scoped package

javan merged commit 50aaf77 into master Aug 30, 2017

javan deleted the node-module branch August 30, 2017 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node module #706

Node module #706

javan commented Aug 28, 2017 •

edited

Loading

gauravtiwari commented Aug 28, 2017

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017

javan commented Aug 29, 2017

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017 •

edited

Loading

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017

javan commented Aug 29, 2017

gauravtiwari Aug 29, 2017

javan Aug 29, 2017

gauravtiwari Aug 29, 2017 •

edited

Loading

gauravtiwari Aug 29, 2017

javan Aug 30, 2017

gauravtiwari Aug 29, 2017

javan Aug 30, 2017

gauravtiwari commented Aug 30, 2017

javan Aug 30, 2017

gauravtiwari Aug 30, 2017

javan Aug 30, 2017

gauravtiwari Aug 30, 2017

javan Aug 30, 2017

gauravtiwari Aug 30, 2017

javan Aug 30, 2017

gauravtiwari Aug 30, 2017

gauravtiwari commented Aug 30, 2017

javan Aug 30, 2017

gauravtiwari Aug 30, 2017

gauravtiwari Aug 30, 2017

javan Aug 30, 2017

Node module #706

Node module #706

Conversation

javan commented Aug 28, 2017 • edited Loading

gauravtiwari commented Aug 28, 2017

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017

javan commented Aug 29, 2017

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017 • edited Loading

javan commented Aug 29, 2017

gauravtiwari commented Aug 29, 2017

javan commented Aug 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gauravtiwari 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

gauravtiwari commented Aug 30, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gauravtiwari commented Aug 30, 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

javan commented Aug 28, 2017 •

edited

Loading

gauravtiwari commented Aug 29, 2017 •

edited

Loading

gauravtiwari Aug 29, 2017 •

edited

Loading