Building elements in a single config #56

rorticus · 2019-05-06T19:11:17Z

Instead of passing a single webpack configuration for each element being built, this is passing a single webpack configuration for all elements. Previously, it would crash if you turned off onlyCompileBundledFiles in ts-loader and ran it on @dojo/widgets because it would run out of memory. In theory this was because each configuration would be loading all the dependencies together and there would be no sharing between them. Remember webpack, sharing is caring!

When running this locally, it'd produce the following structure for @dojo/widgets,

output/
    accordian-pane/
        accordian-pane-1.0.0.js
        accordian-pane-1.0.0.css
    button/
    rinse, lather, repeat for all widgets
    blahblah.svg
    blahblah2.ttf

Compared to the current version of cli-build-webpack, it's missing the dist directory under output/. This shouldn't be difficult to include.
I was having trouble getting file-loader to copy those svg/ttf assets into each widgets directory instead of just leaving them at the root.

I tested that the widgets were being created correctly by quickly creating an example page for the button widget and pulling in the dependencies. Things seemed like they were working fine, but a better test might be to generate everything and then copy the resulting files into the custom element showcase.

agubler · 2019-05-09T11:49:02Z

src/base.config.ts

@@ -115,8 +116,8 @@ export default function webpackConfigFactory(args: any): webpack.Configuration {
 		}, {}),
 		node: { dgram: 'empty', net: 'empty', tls: 'empty', fs: 'empty' },
 		output: {
-			chunkFilename: `[name]-${packageJson.version}.js`,
-			filename: `[name]-${packageJson.version}.js`,
+			chunkFilename: `[name]/[name]-${packageJson.version}.js`,


I was having trouble getting file-loader to copy those svg/ttf assets into each widgets directory instead of just leaving them at the root.

To resolve this issue, shall we drop the folder and have all the elements built at the root, relative to the svg/ttf assets...? (ps don't forget to do the same for the css)

Yeah, if we build everything to the root we don't have an issue. The only problem I can see with that is that you won't have a clear picture of what static assets go with what widgets, since they would no longer be bundled together.

Yeah I think that's fine to be honest - as we won't know what assets are for what widget anyway so would have to blindly copy everything into every folder...

agubler · 2019-05-09T12:21:55Z

@rorticus @dojo/webpack-contrib@6.0.0-alpha.1 is released

rorticus · 2019-05-10T11:21:12Z

OK, flattened out the output directory so everything is built right in output/

rorticus · 2019-05-10T11:28:47Z

Added the dev/dist directories back in.

agubler · 2019-05-10T11:33:18Z

src/base.config.ts

@@ -190,7 +174,12 @@ export default function webpackConfigFactory(args: any): webpack.Configuration {
 						getUMDCompatLoader({ bundles: args.bundles }),
 						{
 							loader: 'ts-loader',
-							options: { onlyCompileBundledFiles: true, instance: jsonpIdent, transpileOnly: true, compilerOptions }
+							options: {
+								onlyCompileBundledFiles: false,


Can we leave this as true until we make the changes for the custom element transformer?

matt-gadd · 2019-05-10T11:34:15Z

src/base.config.ts

+							options: {
+								onlyCompileBundledFiles: false,
+								instance: jsonpIdent,
+								transpileOnly: false,


you can just remove this flag (it defaults to true).

agubler · 2019-05-10T12:19:32Z

package.json

@@ -77,6 +77,7 @@
    "cpx": "~1.5.0",
    "cypress": "3.1.3",
    "execa": "0.8.0",
+    "http-server": "^0.11.1",


Can we pin this dep please?

agubler · 2019-05-10T12:20:44Z

src/main.ts

@@ -223,11 +223,11 @@ const command: Command = {
 		});
 		let configs: webpack.Configuration[];
 		if (args.mode === 'dev') {
-			configs = elements.map((element: any) => devConfigFactory({ ...rc, ...commandLineArgs, element }));
+			configs = [devConfigFactory({ ...rc, ...commandLineArgs, elements })];


Do we need to pass an array of configs any more?

rorticus · 2019-05-10T13:24:58Z

OK, refactored to not pass an array of configs anymore.

codecov · 2019-05-11T01:51:30Z

Codecov Report

Merging #56 into master will increase coverage by 1.39%.
The diff coverage is 50%.

@@            Coverage Diff            @@
##           master     #56      +/-   ##
=========================================
+ Coverage   69.91%   71.3%   +1.39%     
=========================================
  Files           8       8              
  Lines         359     352       -7     
  Branches       62      59       -3     
=========================================
  Hits          251     251              
+ Misses        108     101       -7

Impacted Files	Coverage Δ
src/base.config.ts	`34.92% <ø> (+3.49%)`	⬆️
src/dist.config.ts	`44.44% <0%> (ø)`	⬆️
src/dev.config.ts	`30.76% <0%> (ø)`	⬆️
src/main.ts	`95.23% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 150dc16...e7f787f. Read the comment docs.

agubler · 2019-05-11T07:33:19Z

src/dev.config.ts

 	const outputPath = output!.path as string;

-	config.plugins = [...plugins!, new CleanWebpackPlugin([location], { root: outputPath, verbose: false })];
+	config.plugins = [...plugins!, new CleanWebpackPlugin([outputPath], { root: outputPath, verbose: false })];


Sorry just noticed this, we only want to clean the target for the mode that is being built. For example in this case, path.join(outputPath, 'dev')

@rorticus ^

Building elements in a single config

64433ab

rorticus requested review from agubler and matt-gadd May 6, 2019 19:11

agubler reviewed May 9, 2019

View reviewed changes

agubler mentioned this pull request May 9, 2019

Add --target=lib flag #42

Merged

Flattening out output directory

bba9a21

rorticus marked this pull request as ready for review May 10, 2019 11:21

Putting back the dev/dist directories

92e7e28

agubler reviewed May 10, 2019

View reviewed changes

matt-gadd reviewed May 10, 2019

View reviewed changes

Review feedback

5da8fc3

matt-gadd approved these changes May 10, 2019

View reviewed changes

Adding http-server dependency

1f31456

agubler approved these changes May 10, 2019

View reviewed changes

agubler reviewed May 10, 2019

View reviewed changes

PR feedback

af1ed14

rorticus force-pushed the optimized-elements branch from 95d0750 to af1ed14 Compare May 10, 2019 17:22

Fixing tests

37214ed

agubler reviewed May 11, 2019

View reviewed changes

Cleaning the right directories

e7f787f

rorticus merged commit b1cea44 into dojo:master May 13, 2019

rorticus deleted the optimized-elements branch May 13, 2019 11:48

matt-gadd mentioned this pull request May 15, 2019

Using thread-loader to fix crash when compiling all widgets without onlyCompileBundleFiles #55

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Building elements in a single config #56

Building elements in a single config #56

rorticus commented May 6, 2019

agubler May 9, 2019

rorticus May 9, 2019

agubler May 9, 2019

agubler commented May 9, 2019

rorticus commented May 10, 2019

rorticus commented May 10, 2019

agubler May 10, 2019

rorticus May 10, 2019

matt-gadd May 10, 2019

rorticus May 10, 2019

agubler May 10, 2019

agubler May 10, 2019

rorticus commented May 10, 2019

codecov bot commented May 11, 2019 •

edited

Loading

agubler May 11, 2019

agubler May 11, 2019

Building elements in a single config #56

Building elements in a single config #56

Conversation

rorticus commented May 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agubler commented May 9, 2019

rorticus commented May 10, 2019

rorticus commented May 10, 2019

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

rorticus commented May 10, 2019

codecov bot commented May 11, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 11, 2019 •

edited

Loading