Internal dev watching/rebuilding script and commander interface #36

hutchgrant · 2019-04-09T08:15:09Z

Related Issue

Resolves #25 and #14

Summary of Changes

Whew this is a big list but here it goes...

index.js

index.js, the main entry for the greenwood cli package, is now dedicated as a root control or main() file, with nearly as little individual function calls as possible. It only deals with calling the specific scripts for each mode, based on whatever args are submitted. It also deals with the commander display/control.
most the previous run() functions from the index.js main entry file have been removed. Well where are they? Why would you do that? Well they could be split into 2 categories:
- one being specifically for generating our web components pre-serialization,
- another for actually serializing the application.

So the former category is all we need to call in "dev" mode. We also need to call it not once but twice. Once when we're running the full build (generated components and serializing) and another when we want to run the dev(as well as rebuilding from webpack when a file is changed!).

Therefore, this made sense to have its own file. It is not part of the control of the whole application. It is itself its own specific script. I have added it as generate.js.

I've merged the latter category(serializing) in serialize.js as it was just a single function call, in addition to the main export from serialize.js.

build.js

added an additional function specifically for building the webpack-dev-server internally
hardcoded a webpack-dev-server port of 1981 here because for whatever reason, it won't use the default dev port in the webpack config when I run webpackServer.listen(). Maybe something you want to tweak later.

webpack.config.develop.js

added filewatcher-webpack-plugin with a custom callback onChange. This uses chokidar under the hood, its the same options as chokidar.
had to set a custom debounce. For some reason, the amount of times the onChangeCallback is called, multiplies with each change. I cannot explain what is happening but it fired at least 2 events, once I added the build script it fired more. So I just set a timeout of 1000 ms and it works fine. I left a debug console.log which you can uncomment to see what I mean.

I know what you're thinking, it must be some option you missed etc. I spent a lot of time trying all the options and reading about them to no avail.

Unit Tests

updated to use node ./packages/cli/index.js build

Package.json

I've simply duplicated and slightly modified our outter package.json to prepare it for the individual cli package. It's not being used at the moment.

Known Problems

lit-redux-router will not work on a fresh load of new link. We need a 404.html file to forward back to our index file, in order to get the redux URL change. e.g. a link to a url will work but if you reload the page with a url it will fail. A similar problem will happen in production too(just with 404s not page reload). I have a script to fix it but will have to implement it another time(or now?)
hot module reload doesn't appear to be working. It only worked in Adding Webpack Dev Server #22 because it was restarting dev server, which refreshed the page and gave the appearance of HMR.
need to add a callback to filewatcher plugin for when a new file is added (not just when files are changed)
filewatcher's regex is only looking for .js files in the workspace. Might be a better idea to look for other files as well such as css.
with commander, I set it to use actions rather than options. Perhaps it might be better to use options because at least they'll be listed automatically in help menu. I don't know how to list actions.

e.g. the --help menu:

-------------------------------------------------------
Welcome to Greenwood ♻️
-------------------------------------------------------
Usage: greenwood <script-mode> [options]

Options:
  -V, --version  output the version number
  -h, --help     output usage information

But actually we have:

greenwood build
greenwood dev
greenwood create
greenwood serve

…ix/webpack-errors

…s if alreayd exist

hutchgrant · 2019-04-10T01:10:28Z

Another problem is we would still require a separate generateBuild function because we have to reuse it so we can't have the same script automatically run it be used for importing that to webpack.config.develop.js

…dex.js

thescientist13 · 2019-04-10T02:21:01Z

@hutchgrant
I think we'll be able to make it work. I'll play around with this branch a little bit too, but I think this is a good start here, relative to #22 )

hutchgrant · 2019-04-10T03:32:13Z

Fixed the live-reload! Now back to the other 404 and link reload problem which comes from using routes that rely on redux store. In CEA for example, webpack will automatically route 404s to the index.html file.

 ｢wds｣: Project is running at http://localhost:1981/
ℹ ｢wds｣: webpack output is served from /
ℹ ｢wds｣: 404s will fallback to /index.html

I would like it if we could do that via webpack, but if not, I believe there is a plugin to insert a script into the index.html, specifically we could use it for reloading the url from redux. Also we could include a 404.html and have that bundled only in development(in addition to script insert).

hutchgrant · 2019-04-11T23:28:46Z

Problem with live-reload is compilation var wasn't being reintialized, so we'd have duplicate pages/routes.

The 404 redirect issue in development has been fixed, as well as a default template 404.html has been added in production. Another day we can allow more user control of 404.html

hutchgrant · 2019-04-12T00:18:17Z

There is a problem with the webpack rewrite plugin within webpack.config.common.js, this is related to #32 we're assuming if template is in an import, and src/templates exists, force the import from /src/templates. Which is fine, with page-templates, but if we want to use default app-templates or 404/index templates, that becomes a problem. For now, I've added 404.dev and index.dev to mock-app until issue is resolved. See TODO L11

thescientist13 · 2019-04-12T11:22:05Z

Nice, lot of work here for me to check out! 💪

As a note, and not sure if you have HMR working too, but I think we treat livereload as a different features, given that I see HMR as an extended feature on top of livereload, but not as necessary (in the context of this PR). So if live reload is working, awesome. If HMR isn't working, no worries, let's make it an issue we can work on after this task.

thescientist13 · 2019-04-12T19:47:09Z

packages/cli/config/webpack.config.develop.js


 module.exports = webpackMerge(commonConfig, {

  mode: 'development',

-  devServer: {


could you explain the reasoning for for moving this out of the development config?

when running webpack-dev-server within node, it didn't appear to be doing anything in my initial tests. When you add it as a second parameter in develop.js, it seemed to work.

e.g. toggle hot var in both develop.js and then try it in webpack.config.commons.js using the standard devServer object. You'll notice, turning it on does nothing in the webpack config, but does work in develop.js

ok, let me a take a look at this one. being able to keep it in the webpack config file seems to make the most sense of extending it.

thescientist13

This is looking pretty, pretty, pretty good!

Just left some final comments and I think most can be put into issues, so let me know your thoughts and if you could leave a final comment with any / open items you think we should track, let me know and then we can merge this!!

thescientist13 · 2019-04-12T19:47:47Z

packages/cli/config/webpack.config.develop.js

-  },
+  entry: [
+    'webpack-dev-server/client?http://localhost:1981',
+    path.join(process.cwd(), '.greenwood', 'app', 'app.js')


maybe we should add a TODO or an issue to make sure we go around and clean up any hardcoded paths / strings and pull them from the compilation object?

I just was reminded that there is a TODO above that line in the same file, for the consolidating of the compilation. Do you want to add the TODO to every instance of a hardcoded string? I think its a bit redundant, as long as we have at least one TODO we know there's hardcoded string in that file related to issue #11

edit: woops the common config has it not the develop, which I will add

thescientist13 · 2019-04-12T19:50:25Z

packages/cli/lib/generate.js

+const generateGraph = require('./graph');
+const generateScaffolding = require('./scaffold');
+
+let config = {


anyway to keep this closer to the "top" like in index.js? Having this single source of truth / configuration I think would be beneficial for keeping all the tasks / configs / in lockstep?

As an improvement issue instead, maybe we could put this into a file, and import it in index.js to establish all the defaults, and then pass that around to all the tasks, modifying it based on "userland" configurations in index.js first?

I'd prefer the latter idea. Though functionally it makes sense to have carry through from beginning in index.js but it looks so out of place within that file.

thescientist13 · 2019-04-12T19:55:20Z

package.json

-    "dev-build": "export NODE_ENV=development && yarn build",
-    "dev-watch": "nodemon --watch src --exec 'yarn dev-build'",
-    "dev-serve": "webpack-dev-server --config ./packages/cli/config/webpack.config.develop.js --open",
+    "dev": "node ./packages/cli/index.js dev",


oh, could keep this develop through and through? (task, command, filename?). 😅

If you want, I like less characters to type tbh

thescientist13 · 2019-04-12T19:56:50Z

packages/cli/config/webpack.config.develop.js

+  }
+};
+
+const publicPath = '/';


i see this in prod config too. (publicPath). should we be in webpack.config.common.js?

I agree its a dupe however, where it is used is specific to dev and prod only not both.

That should come from a config/compilation object as well. #11

hutchgrant · 2019-04-12T20:19:01Z

the base for this branch was #22 are we switching it to master then?

thescientist13 · 2019-04-14T20:48:30Z

the base for this branch was #22 are we switching it to master then?

ah, didn't realize that. assumed it was just an alternate version to consider. I will repoint the branch.

thescientist13

Pretty much there, just want to note a couple things:
[ ] Just opened up #39 to do a quick refactor of the webpack dev server config
[x] We should see if we can improve the double index and 404 file maintenance, made #38
[x] Noted some more TODOs in the description of #11 as identified in this PR

and this is good to merge

…de-webpack-config-mini-refactor small webpack config refactor for development mode

thescientist13

🎉

hutchgrant added 29 commits March 27, 2019 17:55

task: adding webpack dev server

9f75e5e

fix: removing outdated imports

f94c3f2

fix: adding file monitor and concurrent monitor+dev-server

ffaf4bd

fix: Webpack Errors not displaying

7d23e1f

Merge branch 'master' into fix/webpack-errors

4bbad8d

Merge branch 'master' into fix/webpack-errors

cba2677

Merge remote-tracking branch 'origin' into task/add-dev-env

8ea44ea

fix: adding missing master unit test script

12c95eb

fix: removing todo

8c1d633

fix: process exits with error

f730416

Merge remote-tracking branch 'origin' into fix/webpack-errors

b4f4a47

fix: fixing error output on exit

2c3f3a8

Merge branch 'master' of github.com:ProjectEvergreen/greenwood into f…

618597c

…ix/webpack-errors

fix: updating lint enabling console output

98f5998

fix: added webpack NormalModuleReplacement for rfc proposal

055ece9

fix: adding styles folder and example

c2498a9

fix: adding front-matter import css example

0833dae

fix: moving theme to styles folder

5e94c2c

fix: updating header component directory demo

7c64a28

fix: updating hello.md theme

538ecd9

fix: remove index.js page and instead use index.md

3df40cd

chore: fixing unit tests

11f8421

fix: removing default index.md fm import

556e0ad

Merge branch 'rfc-user-workspace' into task/add-dev-env

b650c64

fix: fixing re-compile error, don't clean and don't create directorie…

3eb41cf

…s if alreayd exist

fix: re-adding lint script

301d3ba

fix: removing mocha recursive arg

b283f31

Merge remote-tracking branch 'origin' into task/add-dev-env

9907d6d

feat: internal dev watching/rebuilding script and commander interface

8c00cb4

hutchgrant changed the title ~~feat: internal dev watching/rebuilding script and commander interface~~ Internal dev watching/rebuilding script and commander interface Apr 9, 2019

hutchgrant added 2 commits April 9, 2019 22:05

fix: updating file organization to use tasks directory, reorganize in…

545670d

…dex.js

fix: templates and build

50da602

thescientist13 self-assigned this Apr 10, 2019

fix: webpack live-reload

b94e3b4

fix: fixing 404 auto redirect in dev and reinit compilation on rebuild

78593c1

fix: 404.dev and index.dev paths in dev for user workspace

e1aae71

thescientist13 reviewed Apr 12, 2019

View reviewed changes

thescientist13 mentioned this pull request Apr 12, 2019

Adding Webpack Dev Server #22

Closed

fix: changing dev to develop adding TODOs

2acfc79

fix: cli update for develop cmd

1cbfe54

small webpack config refactor

3a44145

thescientist13 changed the base branch from task/add-dev-env to master April 14, 2019 21:04

thescientist13 mentioned this pull request Apr 14, 2019

consolidate configuration and compilation objects #11

Closed

5 tasks

thescientist13 reviewed Apr 14, 2019

View reviewed changes

thescientist13 mentioned this pull request Apr 14, 2019

remove need to maintain prod and development versions of template files #38

Closed

5 tasks

Merge pull request #39 from ProjectEvergreen/task/issue-14-develop-mo…

2be1687

…de-webpack-config-mini-refactor small webpack config refactor for development mode

thescientist13 approved these changes Apr 14, 2019

View reviewed changes

thescientist13 merged commit dc8c18d into master Apr 14, 2019

thescientist13 deleted the task/add-dev-internal-watch-and-commander branch April 14, 2019 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internal dev watching/rebuilding script and commander interface #36

Internal dev watching/rebuilding script and commander interface #36

hutchgrant commented Apr 9, 2019 •

edited

hutchgrant commented Apr 10, 2019

thescientist13 commented Apr 10, 2019

hutchgrant commented Apr 10, 2019

hutchgrant commented Apr 11, 2019 •

edited

hutchgrant commented Apr 12, 2019 •

edited

thescientist13 commented Apr 12, 2019

thescientist13 Apr 12, 2019

hutchgrant Apr 12, 2019 •

edited

thescientist13 Apr 14, 2019

thescientist13 left a comment

thescientist13 Apr 12, 2019

hutchgrant Apr 12, 2019 •

edited

thescientist13 Apr 12, 2019

hutchgrant Apr 12, 2019 •

edited

thescientist13 Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019

thescientist13 Apr 12, 2019

hutchgrant Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019 •

edited

hutchgrant commented Apr 12, 2019

thescientist13 commented Apr 14, 2019

thescientist13 left a comment

thescientist13 left a comment

Internal dev watching/rebuilding script and commander interface #36

Internal dev watching/rebuilding script and commander interface #36

Conversation

hutchgrant commented Apr 9, 2019 • edited

Related Issue

Summary of Changes

index.js

build.js

webpack.config.develop.js

Unit Tests

Package.json

Known Problems

hutchgrant commented Apr 10, 2019

thescientist13 commented Apr 10, 2019

hutchgrant commented Apr 10, 2019

hutchgrant commented Apr 11, 2019 • edited

hutchgrant commented Apr 12, 2019 • edited

thescientist13 commented Apr 12, 2019

thescientist13 Apr 12, 2019

Choose a reason for hiding this comment

hutchgrant Apr 12, 2019 • edited

Choose a reason for hiding this comment

thescientist13 Apr 14, 2019

Choose a reason for hiding this comment

thescientist13 left a comment

Choose a reason for hiding this comment

thescientist13 Apr 12, 2019

Choose a reason for hiding this comment

hutchgrant Apr 12, 2019 • edited

Choose a reason for hiding this comment

thescientist13 Apr 12, 2019

Choose a reason for hiding this comment

hutchgrant Apr 12, 2019 • edited

Choose a reason for hiding this comment

thescientist13 Apr 12, 2019 • edited

Choose a reason for hiding this comment

hutchgrant Apr 12, 2019

Choose a reason for hiding this comment

thescientist13 Apr 12, 2019

Choose a reason for hiding this comment

hutchgrant Apr 12, 2019 • edited

Choose a reason for hiding this comment

hutchgrant Apr 12, 2019 • edited

Choose a reason for hiding this comment

hutchgrant commented Apr 12, 2019

thescientist13 commented Apr 14, 2019

thescientist13 left a comment

Choose a reason for hiding this comment

thescientist13 left a comment

Choose a reason for hiding this comment

hutchgrant commented Apr 9, 2019 •

edited

hutchgrant commented Apr 11, 2019 •

edited

hutchgrant commented Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019 •

edited

thescientist13 Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019 •

edited

hutchgrant Apr 12, 2019 •

edited