[RFC] User Workspace PoC #33

hutchgrant · 2019-04-02T02:54:56Z

Related Issue

Resolves #28

Summary of Changes

ContextReplacementPlugin didn't work, it seems to work only with require('some/library') and not with imports? I couldn't get it to change paths. However, NormalModuleReplacementPlugin does work, it will still likely require some modification, but this is a great start with userland.
I've removed the .md copying, and the component copying. Both will now be rewritten when bundled with webpack to use the user workspace.
Added a custom example with styles, header and navigation components.
Front-matter imports now work as expected. Resolves As a user I should be able to use additional CSS from front matter #8 As a user, I would like to import my own custom assets #27
Removes the need for index.js component page and instead uses index.md

I built this off the webpack error dev branch. This is just proof of concept.

run:

cp ./test/fixtures/mock-app/src ./src -R
yarn serve

go to http://localhost:8000/ to see
- custom app-template with imported components from workspace(see .greenwood scratch)
- index.md instead of index.js
go to http://localhost:8000/hello to see
- custom page-template, with imported css
- markdown component with imported and rendered css(see hello.md)

Problems

I'm aware of a problem with the default template styles replacement. Since we're not importing from a styles directory, the regex will not pick it up and because we're not copying assets/styles, the css file therefore cannot be imported in the default template we supply.

I'm also aware that some of these import replacements may break with the recursive walkdirectory for pages.

…ix/webpack-errors

thescientist13

Looks pretty solid!!

Not a change in this PR, but was reviewing the code in general and found this in init.js
https://github.com/ProjectEvergreen/greenwood/blob/master/packages/cli/lib/init.js#L26

/// set homepage component to user's ./src/component directory instead of def templates folder
config.homeComponent = path.join(process.cwd(), './src/components', 'index.js');

not sure if this problematic, but I wouldn't expect to have a directory like that hardcoded?

Overall, great to see some code going away, always a good sign!

thescientist13 · 2019-04-02T23:41:01Z

package.json

@@ -11,7 +11,8 @@
    "lint": "eslint \"./packages/**/**/*.js\" \"./test/**/**/*.js\"",
    "build": "node ./packages/cli/index.js",
    "serve": "yarn clean && yarn build && cd ./public && ws",
-    "test": "mocha test --timeout 15000"
+    "test": "mocha test --timeout 15000",
+    "prod-test": "webpack --config ./packages/cli/config/webpack.config.prod.js --progress --debug --display verbose"


what's the distinction between this and running yarn build, since that is also running webpack with this same configuration, right?

it was for a quick test, it can be removed.

thescientist13 · 2019-04-02T23:43:01Z

packages/cli/config/webpack.config.common.js

+const fs = require('fs-extra');
+const webpack = require('webpack');
+
+const userLand = path.join(process.cwd(), 'src');


maybe we should standardize our terminologies here.

I think to refer to the user's repository / code, we could call it their workspace? I think userLand is a little too on the 👃 , so to speak 😁

Also, maybe we should make Greenwood's default configuration a file as well, so it can be shared across webpack and nodejs?

Unifying the config, maybe a separate PR and issue?

good call, we have one here #11

thescientist13 · 2019-04-02T23:47:02Z

packages/cli/lib/build.js

-        // TODO webpack errors don't break this task
-        if (err) {
-          console.log(err);
+      return webpack(webpackConfig, (err, stats) => {


I'm pretty sure this change shouldn't be in this PR, right?

it's from the webpack error fix in #23. This PR was a continuation of that branch accidentally.

i guess too late to rollback at this point since this PR is pretty much ready, but will again just appeal to please, please be mindful about conflating unrelated issues / changes, in particular to really intensive PRs like this where anything else is just a distraction, and makes tracking history harder. ✌️

thescientist13 · 2019-04-02T23:53:16Z

packages/cli/config/webpack.config.common.js

@@ -61,6 +81,26 @@ module.exports = {
  },

  plugins: [
+    new webpack.NormalModuleReplacementPlugin(


Interesting, I think that makes sense about ContextReplacementPlugin.

So maybe for this, instead of hardcoding, we "scan" the user's workspace and read all the top level directories, and then use a forEach on all these paths?

This way we won't have to know all the directories ahead time, we literally just provide a mapping mechanism, to essentially create a unified workspace experience. (kind of like mounting volumes with Docker)

Can we do that in a separate PR?

I want to avoid hardcoding though.

That's not a simple fix and is a new feature

that was the goal though?

With this, we can tell webpack where to look when resolving paths so that Greendwood can dynamically look up and link to a user's entire project without needing to configure everything other than

🤷‍♂️

i say it's the same feature, just refactored to be dyanmic instead of hardcoded? I'll see if i can give it a try, I think it's worth the effort given the payoff, IMO,

True, but for now, it's best to just make sure a few predetermined directories work(which they now do). You'll have to walk the src directory(if it exists), then create an array of plugins(one for each folder), with a unique regex and replacement. You'll have to do some string manipulation, test it all, etc.

A flexible user workspace through import or @import
components/
assets/
styles/
whatever/!

Is simpler to start with.

I mean the function that does that will be useful, if no src directory exists, you'll want a specific replacement for styles for the default template.

whatever/!

that was meant imply it was supposed to be dynamic but no problem, I think I'm pretty close to implementing the needed change.

thescientist13 · 2019-04-02T23:54:40Z

.eslintrc

@@ -10,7 +10,7 @@
  "rules": {
    "comma-dangle": [2,"never"],
    "no-cond-assign": 2,
-    "no-console": 1,
+    "no-console": 0,


yeah, my plan to handle this was to implement a logging package or something in #15

for now its just throwing unnecessary warnings so I disabled it.

understood, but there's already an issue for handling that and this still leaves all those // eslint disable line comments.

just in general, let's try and keep PRs as focused as possible to the immediate task at hand, in particular if there is an existing and overlapping issue open. 👍

right but even with that PR implemented, we will still have to deal with the lint issue(which isn't mentioned in that issue).

well, not debate it or anything, but in the issue I proposed we add a logging library, which would then be a dependency in our packages, e.g.

const LOG_LEVEL = isDebug ? 0 : 2; const logger = require('logger')(LOG_LEVEL);

and not a console anymore and allow us to programmatically set logging levels, etc.

anyway, the main takeaway here is just avoid cross pollinating is all, please.

thescientist13 · 2019-04-03T01:27:59Z

test/fixtures/mock-app/src/styles/template.css

@@ -0,0 +1,39 @@
+:host {


does CSS actually work in our build right now? or are these tests here just for testing the directory mapping with webpack?

css does work, only in userworkspace

neat! looks we should start building a website soon with this or something 🙃

thescientist13

refactored webpack here #33 . I think we can merge that, and then this should mostly be ready to merge as well.

thescientist13

Awesome stuff, excited to merger this in, didn't think we would solve it so fast!! 👏

hutchgrant added 10 commits March 27, 2019 19:26

fix: Webpack Errors not displaying

7d23e1f

Merge branch 'master' into fix/webpack-errors

4bbad8d

Merge branch 'master' into fix/webpack-errors

cba2677

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

hutchgrant requested a review from thescientist13 April 2, 2019 02:59

hutchgrant added 8 commits April 1, 2019 23:08

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

thescientist13 reviewed Apr 3, 2019

View reviewed changes

hutchgrant added 3 commits April 2, 2019 20:14

fix: removing unnecessary script

027029e

fix: changing path terminology to userWorkspace

3e5f8fe

fix: removing deprecated homeComponent

b0974e8

thescientist13 mentioned this pull request Apr 3, 2019

refactor workspace mapping #34

Merged

thescientist13 added the RFC Proposal and changes to workflows, architecture, APIs, etc label Apr 3, 2019

thescientist13 reviewed Apr 3, 2019

View reviewed changes

thescientist13 mentioned this pull request Apr 3, 2019

consolidate configuration and compilation objects #11

Closed

5 tasks

refactor workspace mapping (#34)

3516404

thescientist13 approved these changes Apr 3, 2019

View reviewed changes

thescientist13 merged commit 7f569ab into master Apr 3, 2019

thescientist13 deleted the rfc-user-workspace branch April 3, 2019 01:42

This was referenced Apr 3, 2019

Fixing Webpack Error Display #23

Closed

webpack not displaying errors when running from node #10

Closed

hutchgrant mentioned this pull request Apr 3, 2019

Recursive Nested Page Generation #35

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] User Workspace PoC #33

[RFC] User Workspace PoC #33

hutchgrant commented Apr 2, 2019 •

edited

thescientist13 left a comment

thescientist13 Apr 2, 2019

hutchgrant Apr 3, 2019

thescientist13 Apr 2, 2019

thescientist13 Apr 3, 2019

hutchgrant Apr 3, 2019

thescientist13 Apr 3, 2019

thescientist13 Apr 2, 2019

hutchgrant Apr 3, 2019 •

edited

thescientist13 Apr 3, 2019

thescientist13 Apr 2, 2019

hutchgrant Apr 3, 2019

thescientist13 Apr 3, 2019

hutchgrant Apr 3, 2019

thescientist13 Apr 3, 2019

thescientist13 Apr 3, 2019

hutchgrant Apr 3, 2019 •

edited

hutchgrant Apr 3, 2019

thescientist13 Apr 3, 2019

thescientist13 Apr 2, 2019

hutchgrant Apr 3, 2019

thescientist13 Apr 3, 2019

hutchgrant Apr 3, 2019 •

edited

thescientist13 Apr 3, 2019 •

edited

thescientist13 Apr 3, 2019

hutchgrant Apr 3, 2019

thescientist13 Apr 3, 2019

thescientist13 left a comment

thescientist13 left a comment •

edited

[RFC] User Workspace PoC #33

[RFC] User Workspace PoC #33

Conversation

hutchgrant commented Apr 2, 2019 • edited

Related Issue

Summary of Changes

Problems

thescientist13 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hutchgrant Apr 3, 2019 • edited

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

hutchgrant Apr 3, 2019 • edited

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

hutchgrant Apr 3, 2019 • edited

Choose a reason for hiding this comment

thescientist13 Apr 3, 2019 • edited

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

thescientist13 left a comment

Choose a reason for hiding this comment

thescientist13 left a comment • edited

Choose a reason for hiding this comment

hutchgrant commented Apr 2, 2019 •

edited

hutchgrant Apr 3, 2019 •

edited

hutchgrant Apr 3, 2019 •

edited

hutchgrant Apr 3, 2019 •

edited

thescientist13 Apr 3, 2019 •

edited

thescientist13 left a comment •

edited