Webpack loader #186

zamotany · 2017-12-13T13:42:49Z

Documentation is missing, but I'll make it once the code will be done.

So the JS rule looks like this:

{
  test: /\.js$/,
  use: [
    { loader: 'babel-loader', options: { cacheDirectory: true } },
    { loader: 'linaria/loader' }
  ],
},

Then while testing on website:

changing theme.js rebuilds CSS so HMR updates styles
babel cache is not a problem anymore
deleting .linaria-cache doesn't break anything

codecov · 2017-12-13T13:46:12Z

Codecov Report

Merging #186 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #186   +/-   ##
=======================================
  Coverage   97.64%   97.64%           
=======================================
  Files          19       19           
  Lines         425      425           
  Branches       90       90           
=======================================
  Hits          415      415           
  Misses         10       10

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 3b894e5...e124b4b. Read the comment docs.

thymikee · 2017-12-13T15:06:17Z

loader.js

@@ -0,0 +1 @@
+module.exports = require('./build/tools/webpack-loader').default;


We want to publish this file, please add it to "files" in package.json

satya164 · 2017-12-14T02:03:22Z

website/webpack.config.js

@@ -50,7 +50,7 @@ module.exports = (env = { NODE_ENV: 'development' }) => ({
      {
        test: /\.js$/,
        exclude: /node_modules/,
-        use: { loader: 'babel-loader' },
+        use: [{ loader: 'babel-loader' }, { loader: 'linaria/loader' }],


Let's enable cache for babel-loader here to make sure we always test against it

satya164 · 2017-12-14T02:08:27Z

src/tools/webpack-loader.js

+  };
+}
+
+function makeLoaderAdapter(fn) {


This abstraction seems unnecessary. Why not call the function directly instead of creating a higher order function to which you pass it as an argument? We don't seem to need it to reuse anywhere.

I got it done, before I found a perfect solution and back them the whole loader was async making it harder to integrate with webpack.

satya164 · 2017-12-14T02:10:35Z

src/tools/webpack-loader.js

+
+  // `transformFromAst` is synchronous in Babel 6, but async in Babel 7 hence
+  // the `transformFromAstSync`.
+  return (babel.transformFromAstSync || babel.transformFromAst)(


Probably better to use the async babel.transformFromAst

There is no async version of transformFromAst in Babel 6.

Yes, I meant always use babel.transformFromAst, sync or async. You were handling promises anyway.

satya164 · 2017-12-14T02:22:42Z

src/tools/webpack-loader.js

+}
+
+function makeLoaderAdapter(fn) {
+  function loaderAdapter(...args: any[]) {


This code can be hugely simplified by making the result always async:

const callback = this.async(); Promise.resolve() .then(() => fn.call(this, ...args)) .then( ({ source, map, meta }) => callback(null, source, map, meta), error => callback(error) );

In this case, it's better to use the second argument of .then here for the error callback instead of using .catch so that if callback throws error, it doesn't get caught and passed to callback again. At least that doesn't seem like the intended use of the API.

satya164 · 2017-12-14T02:28:42Z

src/tools/webpack-loader.js

+        return;
+      }
+
+      const source = fs.readFileSync(reason.module.resource).toString();


Better to do fs.readFileSync(reason.module.resource, 'utf8'); instead of .toString(), though not sure if webpack's memory fs supports it.

It supports it, but is there any actual reason to change it?

satya164 · 2017-12-14T02:32:22Z

src/tools/webpack-loader.js

+}
+
+function transpile(source: string, map: any, filename: string) {
+  const file = new babel.File(


What happens if the user doesn't have .babelrc file at all?

It uses normal .babelrc/package.json/.babelrc.js resulution, so if user don't have a babel config it will just assume that the code is standard ES2015 without any experimental features.

As for inlining config in babel-loader, if it's possible to get the options from it through webpack, I do that, otherwise, the loader will accept inlined babel config.

satya164 · 2017-12-14T10:44:22Z

src/tools/webpack-loader.js

+      filename,
+      sourceMaps: true,
+      inputSourceMap: map,
+      presets: [require.resolve('../../babel.js')],


We should be able to pass options to our babel preset via the loader options

Hmm, shouldn't it be presets: [require.resolve('../../babel.js'), loaderOptions] ?

Why? There are other options for babel that you might be interested in. Limiting it only to presets doesn't sound good. What if you want to pass plugins?

Still, if you want to pass options to linaria preset it would look like this:

{ loader: 'linaria/loader', options: { presets: [['linaria/babel', { /* options */ }]] } }

Why would we want to allow passing arbitrary presets and plugins? It's linaria-loader. People can use babel-loader for other babel stuff. Otherwise it'll just be a half-baked babel-loader with linaria preset included by default.

But maybe you don't want all of out plugins to be applied? Only preval-extract?

Not sure what's the usecase, but it should be an item in the options object if it needs to be exposed instead of overriding all plugins/presets.

satya164

Now that we have the loader, I think it's worth adding to the README that you don't need to be using Babel to use Linaria if you use the webpack loader.

satya164 · 2017-12-14T12:26:28Z

docs/BABEL_PRESET.md

@@ -1,5 +1,9 @@
 # `linaria/babel` preset

+__If you use Webpack__, we highly recommend to add `linaria/loader` to your JS rule in Webpack config. It supports the same options as the preset. For more information go [here](./BUNDLERS_INTEGRATION.md#loader).


I think the term JS rule is confusing. I'd remove it since we have clear instructions for this anyway.

Add to avoid issues to the end of the first line.

Change For more information go [here] to [See here] for instructions on configuring the loader.

satya164 · 2017-12-14T12:27:48Z

docs/BUNDLERS_INTEGRATION.md

@@ -2,6 +2,54 @@

 ## webpack

+### Loader
+
+For the best user experience while working with Linaria we recommend to use our custom loader.


For the best developer experience with Linaria, we recommend to use our webpack loader`

satya164 · 2017-12-14T12:30:02Z

docs/BUNDLERS_INTEGRATION.md

+
+For the best user experience while working with Linaria we recommend to use our custom loader.
+
+Just add it to a JS rule after `babel-loader` in webpack config:


In your webpack config, you'll need to add `linaria/loader` to run Linaria on `.js` files:

satya164 · 2017-12-14T12:33:27Z

docs/BUNDLERS_INTEGRATION.md

+},
+```
+
+It serves the following purposes:


I think it's unnecessary to mention these since these were bugs/limitations.

zamotany added 2 commits December 11, 2017 14:36

webpack loader MVP

1dfbf8f

get parser options by creating File instance

44bb724

zamotany requested review from satya164 and thymikee December 13, 2017 13:42

thymikee reviewed Dec 13, 2017

View reviewed changes

satya164 requested changes Dec 14, 2017

View reviewed changes

zamotany added 4 commits December 14, 2017 10:20

use babel-loader's cache

d104ae5

add loader.js to published files list

5b632bd

adress requested changes

7adcf67

get inlined config from babel-loader's options

b9ca27a

satya164 requested changes Dec 14, 2017

View reviewed changes

zamotany added 2 commits December 14, 2017 11:49

allow to pass options when transpiling

4632b2d

pass loader options to linraia babel preset

ca187d9

satya164 approved these changes Dec 14, 2017

View reviewed changes

update docs

0ce4c4c

satya164 requested changes Dec 14, 2017

View reviewed changes

zamotany and others added 2 commits December 14, 2017 13:50

update docs

aa3d2a9

Update README.md

e124b4b

satya164 approved these changes Dec 14, 2017

View reviewed changes

zamotany merged commit 4efecba into master Dec 14, 2017

zamotany deleted the webpack-loader branch December 14, 2017 12:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webpack loader #186

Webpack loader #186

zamotany commented Dec 13, 2017 •

edited

Loading

codecov bot commented Dec 13, 2017 •

edited

Loading

thymikee Dec 13, 2017

zamotany Dec 14, 2017

satya164 Dec 14, 2017

satya164 Dec 14, 2017

zamotany Dec 14, 2017

satya164 Dec 14, 2017

zamotany Dec 14, 2017

satya164 Dec 14, 2017 •

edited

Loading

satya164 Dec 14, 2017

satya164 Dec 14, 2017

zamotany Dec 14, 2017

satya164 Dec 14, 2017

zamotany Dec 14, 2017

satya164 Dec 14, 2017 •

edited

Loading

zamotany Dec 14, 2017

satya164 Dec 14, 2017

zamotany Dec 14, 2017

zamotany Dec 14, 2017

satya164 Dec 14, 2017 •

edited

Loading

zamotany Dec 14, 2017

satya164 Dec 14, 2017

satya164 left a comment

satya164 Dec 14, 2017

satya164 Dec 14, 2017

satya164 Dec 14, 2017

satya164 Dec 14, 2017

		@@ -0,0 +1 @@
		module.exports = require('./build/tools/webpack-loader').default;

		@@ -1,5 +1,9 @@
		# `linaria/babel` preset

		__If you use Webpack__, we highly recommend to add `linaria/loader` to your JS rule in Webpack config. It supports the same options as the preset. For more information go [here](./BUNDLERS_INTEGRATION.md#loader).


		For the best user experience while working with Linaria we recommend to use our custom loader.

		Just add it to a JS rule after `babel-loader` in webpack config:

Webpack loader #186

Webpack loader #186

Conversation

zamotany commented Dec 13, 2017 • edited Loading

codecov bot commented Dec 13, 2017 • edited Loading

Codecov Report

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

satya164 Dec 14, 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

Choose a reason for hiding this comment

satya164 Dec 14, 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

satya164 Dec 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satya164 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

zamotany commented Dec 13, 2017 •

edited

Loading

codecov bot commented Dec 13, 2017 •

edited

Loading

satya164 Dec 14, 2017 •

edited

Loading

satya164 Dec 14, 2017 •

edited

Loading

satya164 Dec 14, 2017 •

edited

Loading