Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: apply webpack-defaults #11

Merged
merged 17 commits into from
Mar 16, 2017
Merged

refactor: apply webpack-defaults #11

merged 17 commits into from
Mar 16, 2017

Conversation

jhnns
Copy link
Member

@jhnns jhnns commented Mar 13, 2017

This PR introduces a breaking change.

This refactoring adds major changes to the loader:

  • Remove node 0.10 and node 0.12 support
  • The loaded module must now export a function
  • This function will be called with the loader options
  • This function must return an object with this structure
<outdated, see below>
  • The loaded module may flag it's generated value as cacheable
{
    cacheable: true|false,
    ...
}
  • The loaded module may also return an array with dependencies that need to be tracked
{
    dependencies: [
        require.resolve('./a.js'),
        require.resolve('./b.js')
    ],
    ...
}
  • The function may also return a promise for async results

More details in the README.md

jhnns added 11 commits March 11, 2017 02:39
This feature is not available in webpack for a long time now.
- Remove 0.10 and 0.12 node support
- Expect loaded module to export a single function instead of the actual value
- The exported function will be called with the loader options
- The returned value should be an object that also provides meta information like cacheable or dependencies
- The returned value can also contain source maps
- Add support for async results
- Adjust linting rules for node v4
@codecov
Copy link

codecov bot commented Mar 13, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@d873c86). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff           @@
##             master    #11   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?      1           
  Lines             ?     26           
  Branches          ?      7           
=======================================
  Hits              ?     26           
  Misses            ?      0           
  Partials          ?      0
Impacted Files Coverage Δ
src/index.js 100% <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 d873c86...5b51659. Read the comment docs.

@bebraw
Copy link

bebraw commented Mar 13, 2017

What would you think of jumping to webpack-defaults?

@jhnns
Copy link
Member Author

jhnns commented Mar 13, 2017

Yeah, I will take a look tomorrow 👍

@jhnns
Copy link
Member Author

jhnns commented Mar 13, 2017

Switching to webpack-defaults was pretty easy 👍

Please do not merge this PR yet, I haven't finished the README

Separate the expected `value` array into more meaningful properties like `code`, `sourceMap`, `ast`.
@jhnns jhnns mentioned this pull request Mar 14, 2017
@jhnns
Copy link
Member Author

jhnns commented Mar 14, 2017

I've refactored the proposed API. Excerpt from the README:


Examples

If you have a module findAnswer.js like this:

function findAnswer() {
    return {
        code: 'module.exports = 42;'
    };
}

module.exports = findAnswer;

you can use the val-loader to generate source code on build time:

// webpack.config.js
module.exports = {
    ...
    module: {
        rules: [{
            test: require.resolve('path/to/findAnswer.js'),
            use: [{
                loader: 'val-loader'
            }]
        }]
    }
};

A complete example of all available features looks like this:

const ask = require('./ask.js');
const generateResult = require('./generateResult.js');

function findAnswer(options) {
    return ask(options.question)
        .then(generateResult)
        .then(result => ({
            
            code: result.code,
            sourceMap: result.sourceMap,
            ast: result.abstractSyntaxTree,
            
            // Mark dependencies of findAnswer().
            // The function will be re-executed if one of these
            // dependencies has changed in watch mode.
            dependencies: [
                // Array of absolute native paths!
                require.resolve('./ask.js'),
                require.resolve('./generateResult.js')
            ],
            
            // Flag the generated code as cacheable.
            // If none of the dependencies have changed,
            // the function won't be executed again.
            cacheable: true

        }));
}

module.exports = findAnswer;
// webpack.config.js
module.exports = {
    ...
    module: {
        rules: [{
            test: require.resolve('path/to/findAnswer.js'),
            use: [{
                loader: 'val-loader',
                options: {
                    question: 'What is the meaning of life?'
                }
            }]
        }]
    }
};

Usage

The module that is loaded with this loader must stick to the following interfaces:

Expected module interface

The loaded module must export a function as default export with the following function interface.

module.exports = function (...) { ... };

Modules transpiled by Babel are also supported.

export default function (...) { ... }

Expected function interface

The function will be called with the loader options and must either return:

  • an object with the following object interface or
  • a promise that resolves to the following object interface

Expected object interface

Property Type Description
code `string Buffer`
sourceMap SourceMap Optional. Will be pased to the next loader or to webpack.
ast  any Optional. An Abstract Syntax Tree that will be passed to the next loader. Useful to speed up the build time if the next loader uses the same AST.
dependencies Array<string> Default: []. An array of absolute, native paths to file dependencies that need to be watched for changes.
cacheable boolean Default: false. Flag whether the code can be re-used in watch mode if none of the dependencies have changed.

Loader Options

The val-loader itself has no options. The options are passed as they are (without cloning them) to the exported function.

@jhnns
Copy link
Member Author

jhnns commented Mar 14, 2017

This PR is now mergeable if you're ok with it.

@bebraw
Copy link

bebraw commented Mar 14, 2017

Only Travis is weird. I wonder if that's something specific in webpack-defaults.

@jhnns
Copy link
Member Author

jhnns commented Mar 14, 2017

Yeah, Travis looks broken ^^ I don't get it...

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Mar 14, 2017

We're experiencing issues processing builds for pull requests - Travis CI Status

Yesterday the build run for hours, not sure why and if it's related to problems @travis, maybe wait 1-2 days 😛

@bebraw
Copy link

bebraw commented Mar 14, 2017

Based on this error I would say Node 4 is missing some feature you depend on. Fixing that might not fix the whole build, though.

@joshwiens
Copy link
Member

@jhnns - The crux of the problem was that the existing travis.yml and the new content were merged & not overwritten ( //cc @sapegin - this should work like the .babelrc )

The merged output was trying to run an OSX matrix which is blocking / can block other build execution which is why it just sat there forever.

@joshwiens
Copy link
Member

For reference, whatever the issue is it's specific to NodeJS v4.3 & v4.4

Everything works as expected on NodeJS >=4.5

@jhnns
Copy link
Member Author

jhnns commented Mar 15, 2017

@jhnns - The crux of the problem was that the existing travis.yml and the new content were merged & not overwritten ( //cc @sapegin - this should work like the .babelrc )

The merged output was trying to run an OSX matrix which is blocking / can block other build execution which is why it just sat there forever.

That makes sense, thanks for spotting this. I didn't take a close look to be honest 😁

I will fix the buffer issue today.

@bebraw
Copy link

bebraw commented Mar 15, 2017

The Travis issue should be fixed in webpack-defaults now.

@jhnns jhnns merged commit caf2aab into master Mar 16, 2017
@jhnns jhnns deleted the refactor branch March 16, 2017 15:06
@jhnns
Copy link
Member Author

jhnns commented Mar 16, 2017

Only codecov was reporting that there was no coverage report found to compare, so this is safe to merge now. Next step: release

@joshwiens joshwiens mentioned this pull request Apr 9, 2017
@joshwiens joshwiens changed the title Refactor refactor: apply webpack-defaults Apr 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants