Vue preset #484

helfi92 · 2017-11-22T14:39:16Z

No description provided.

helfi92 · 2017-11-22T19:30:25Z

packages/neutrino/src/api.js

  }, opts);

+  Object.defineProperty(options, 'extensions', {


@eliperelman Let me know if this is the way you wanted it to be done. Once we've agreed on a way, I could update the rest neutrino to use it.

As discussed in IRC, this can just be a normal property, and consumers can access the Set directly. We can also expose a method for generating a regex on the fly, but we don't want to limit this property to only creating regexes.

Another thing, a user should be able to pass this as an array to Neutrino options, which when merged using mergeOptions or whatever, can be merged with the Set values:

module.exports = { options: { extensions: ['vue'] } }; // ... new Set([...this.options.extensions, ...extensions])

When merging we should also probably normalize the values to ensure they don't contain dots.

eliperelman · 2017-11-27T17:56:43Z

packages/vue/README.md

+Time: 9773ms
+                           Asset       Size    Chunks             Chunk Names
+   index.dfbad882ab3d86bfd747.js     181 kB     index  [emitted]  index
+polyfill.57dabda41992eba7552f.js    69.2 kB  polyfill  [emitted]  polyfill


Remove this polyfill line.

eliperelman · 2017-11-27T18:00:33Z

packages/vue/index.js

+
+  neutrino.config.when(neutrino.config.module.rules.has('lint'), () => neutrino.config.module
+    .rule('lint')
+    .test(neutrino.options.extensions)


Use the loader merge here:

neutrino.config.when(neutrino.config.module.rules.has('lint'), () => { neutrino.use(loaderMerge('lint', 'eslint'), { plugins: ['vue'], envs: ['node'], parserOptions: { ecmaFeatures: { jsx: true } }, rules: { 'vue/jsx-uses-vars': 2 } }); });

eliperelman · 2017-11-27T18:01:45Z

packages/vue/package.json

+  "dependencies": {
+    "@neutrinojs/web": "^7.3.2",
+    "babel-preset-vue": "^1.2.1",
+    "deepmerge": "^1.4.4",


Not sure where this version came from, but everywhere else it's at ^1.5.2.

eliperelman · 2017-11-27T18:02:25Z

packages/vue/package.json

+    "eslint-plugin-vue": "^2.1.0",
+    "stylelint-processor-html": "^1.0.0",
+    "vue-loader": "^13.5.0",
+    "vue-template-compiler": "^2.5.6"


Is this a peerDependency of something? I don't see this used anywhere.

Yeah, it's a peerDependency of vue-loader https://github.com/vuejs/vue-loader/blob/master/package.json#L60.

eliperelman · 2017-11-27T18:04:50Z

packages/vue/index.js

+module.exports = (neutrino, options) => {
+  neutrino.use(web, options);
+
+  neutrino.config.module.rule('vue')


Use indented chaining when contexts change:

neutrino.config.module .rule('vue') .test(neutrino.options.extensions) .use('vue') .loader(require.resolve('vue-loader')) .options(options);

eliperelman · 2017-11-27T18:06:57Z

packages/neutrino/src/api.js

  }, opts);

+  Object.defineProperty(options, 'extensions', {


Another thing, a user should be able to pass this as an array to Neutrino options, which when merged using mergeOptions or whatever, can be merged with the Set values:

module.exports = { options: { extensions: ['vue'] } }; // ... new Set([...this.options.extensions, ...extensions])

When merging we should also probably normalize the values to ensure they don't contain dots.

helfi92 · 2017-11-28T18:17:31Z

@eliperelman You mention here to have extensions merged i.e., new Set([...this.options.extensions, ...extensions]). I'm not sure why we need to merge the extensions. Sometimes you want to overwrite the entries instead of appending to the set. Let me know what you think about my last commit.

eliperelman · 2017-11-28T18:48:40Z

I'm not sure why we need to merge the extensions. Sometimes you want to overwrite the entries instead of appending to the set.

I see this as being the primary use case:

module.exports = {
  options: {
    extensions: ['esm']
  }
};

Basically, a user can come in and add their own extensions, without having to replicate the whole list of other extensions that we store. Hence needing to merge instead of replace.

A replacement can still happen though if you're using the API, which seems good because I think it's a less common need, and only happens in more advanced cases:

neutrino.extensions = new Set(['js', 'jsx']);

* Expose a method for generating a regex from extensions * Use indented chaining * Use an updated version of the deepmerge package

helfi92 · 2017-11-29T01:57:04Z

@eliperelman, In my last commit, something like the following:

module.exports = {
  options: {
    extensions: ['.vue', 'vue'],
  },
  ...
};

would make neutrino.options.extensions equal [ 'js', 'jsx', 'vue', 'ts', 'mjs', 'vue' ] whereas something like neutrino.options.extensions = ['vue'] overwrites the default extensions. Does it make more sense now? Thanks for your patience.

eliperelman

Looks great, I think we're close! Have you tested that merging extensions from .neutrinorc.js works correctly?

eliperelman · 2017-11-29T13:17:09Z

packages/neutrino/src/api.js

+    set(extensions) {
+      const newExtensions = extensions.map(ext => equals(ext[0], '.') ? tail(ext) : ext);
+
+      fileExtensions = [...new Set(newExtensions)];


Looks like the original value of fileExtensions is a Set, but here we spread it into an array. We can keep this as a Set here, and only spread to array during get:

fileExtensions = new Set(newExtensions);

Combining this with my previous comment, this can be one-lined into:

fileExtensions = new Set(extensions.map(ext => ext.replace('.', '')));

eliperelman · 2017-11-29T13:18:25Z

packages/neutrino/src/api.js

+      return [...fileExtensions];
+    },
+    set(extensions) {
+      const newExtensions = extensions.map(ext => equals(ext[0], '.') ? tail(ext) : ext);


This could be simplified to:

const newExtensions = extensions.map(ext => ext.replace('.', ''));

eliperelman · 2017-11-29T13:19:51Z

packages/neutrino/src/api.js

@@ -26,14 +27,28 @@ const pathOptions = [

 // getOptions :: Object? -> IO Object
 const getOptions = (opts = {}) => {
+  let fileExtensions = new Set(['js', 'jsx', 'vue', 'ts', 'mjs']);


Let's rename this to moduleExtensions so it's clear we are only dealing with module code instead of things like images and stylesheets.

eliperelman · 2017-11-29T13:20:07Z

packages/neutrino/src/api.js

@@ -87,6 +102,10 @@ class Api {
    this.config = new Config();
  }

+  regexFromExtensions(extensions = this.options.extensions) {
+    return new RegExp(`.(${extensions.join('|')})$`);


eliperelman · 2017-11-29T13:27:31Z

packages/vue/index.js

+
+module.exports = (neutrino, options) => {
+  neutrino.use(web, options);
+  neutrino.options.extensions = ['vue']; // eslint-disable-line no-param-reassign


We now have vue in the extensions list, so we don't need this line now.

If I include the js extension, I get an error:

ERROR in Error: The loader "/Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/html-webpack-plugin/lib/loader.js!/Use rs/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/html-webpack-template/index.ejs" didn't return html.

If extensions is something like ['jsx', 'vue', 'ts', 'mjs' ] (without the js extension), then it works. Do you know why this is happening?

Also, should we include .json in the list of extensions? :)

Looks like this:

jantimon/html-webpack-plugin#602

Basically, I think the vue-loader is only supposed to accept vue extensions:

neutrino.config.module .rule('vue') .test(/\.vue$/) // instead of .test(neutrino.regexFromExtensions()) .use('vue') .loader(require.resolve('vue-loader')) .options(options);

Also, should we include .json in the list of extensions? :)

I...don't know. Give it a shot and see if the tests continue to pass, but I'm not sure what effect it may have.

That's what I figured, sounds good!

Btw, instead of hardcoding /\.vue$/, we could just do neutrino.regexFromExtensions(['vue']).

eliperelman · 2017-11-29T13:28:17Z

packages/vue/index.js

+const web = require('@neutrinojs/web');
+const path = require('path');
+const merge = require('deepmerge');
+const loaderMerge = require('@neutrinojs/loader-merge');


nit: move this require to be above the web one.

eliperelman · 2017-11-29T13:29:36Z

packages/vue/index.js

+  neutrino.config
+    .resolve
+      .modules.add(MODULES).end()
+      .extensions.add('.vue').end()


We should make this automatic from the web preset:

neutrino.config.resolve.extensions.merge(neutrino.options.extensions.map(ext => `.${ext}`);

Something like that.

helfi92 · 2017-11-29T14:44:07Z

@eliperelman Adding .json to the list of extensions seems to work. Added the requested changes. Hopefully, things are neater :) Once that is done, I could clean the other packages to use the list of extensions. Also, to answer your question, I have tested merging extensions from .neutrinorc.js. The example shown earlier was the one I tried on my machine.

eliperelman · 2017-11-29T15:48:39Z

packages/neutrino/src/api.js

  const options = merge({
    env: {
      NODE_ENV: 'development'
    },
    debug: false,
-    quiet: false
+    quiet: false,
+    extensions: moduleExtensions


Remove this line.

eliperelman

See comments.

…-preset

eliperelman

🎉 🌮 🎉

barraponto and others added 2 commits November 22, 2017 09:27

Add untouched barraponto vue preset

2d446bf

Modify preset to comply with new neutrino

b47adff

helfi92 self-assigned this Nov 22, 2017

helfi92 mentioned this pull request Nov 22, 2017

Add vue preset #457

Closed

helfi92 added 6 commits November 22, 2017 09:45

Use absolute URL in README

884a0af

Add vue test

1b803e3

Add vue npmignore

dacdc59

Remove confusing compileRule

147a6b7

Introduce API option: extensions

578753b

Start using neutrino.options.extensions

7a79aa1

helfi92 requested a review from eliperelman November 22, 2017 17:47

helfi92 commented Nov 22, 2017

View reviewed changes

eliperelman suggested changes Nov 27, 2017

View reviewed changes

helfi92 requested a review from eliperelman November 28, 2017 18:17

Add a method to API to get regex from extensions

688f53a

* Expose a method for generating a regex from extensions * Use indented chaining * Use an updated version of the deepmerge package

helfi92 force-pushed the vue-preset branch from ef8b053 to 688f53a Compare November 29, 2017 01:48

eliperelman suggested changes Nov 29, 2017

View reviewed changes

Add .json to list of extensions

863fa59

helfi92 requested a review from eliperelman November 29, 2017 14:44

eliperelman added 2 commits November 29, 2017 09:44

Merge branch 'master' into vue-preset

a7b138d

Merge branch 'master' into vue-preset

f01c2c6

eliperelman reviewed Nov 29, 2017

View reviewed changes

Remove unneeded line

2b0e622

eliperelman suggested changes Nov 29, 2017

View reviewed changes

helfi92 added 2 commits November 29, 2017 10:55

Copy vue's README to docs

7301a1f

Merge branch 'vue-preset' of github.com:helfi92/neutrino-dev into vue…

4733bb8

…-preset

helfi92 requested a review from eliperelman November 29, 2017 16:16

eliperelman approved these changes Nov 29, 2017

View reviewed changes

helfi92 merged commit 3d253ae into neutrinojs:master Nov 29, 2017

eliperelman added the feature label Nov 29, 2017

eliperelman mentioned this pull request Nov 30, 2017

Use preact and vue in create-neutrino-project #442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vue preset #484

Vue preset #484

helfi92 commented Nov 22, 2017

helfi92 Nov 22, 2017

eliperelman Nov 27, 2017

eliperelman Nov 27, 2017

eliperelman Nov 27, 2017

eliperelman Nov 27, 2017

eliperelman Nov 27, 2017

eliperelman Nov 27, 2017

helfi92 Nov 28, 2017

eliperelman Nov 27, 2017

eliperelman Nov 27, 2017

helfi92 commented Nov 28, 2017

eliperelman commented Nov 28, 2017

helfi92 commented Nov 29, 2017 •

edited

Loading

eliperelman left a comment

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

helfi92 Nov 29, 2017

helfi92 Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

helfi92 Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman Nov 29, 2017

helfi92 commented Nov 29, 2017

eliperelman Nov 29, 2017

eliperelman left a comment

eliperelman left a comment

Vue preset #484

Vue preset #484

Conversation

helfi92 commented Nov 22, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helfi92 commented Nov 28, 2017

eliperelman commented Nov 28, 2017

helfi92 commented Nov 29, 2017 • edited Loading

eliperelman 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

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

helfi92 commented Nov 29, 2017

Choose a reason for hiding this comment

eliperelman left a comment

Choose a reason for hiding this comment

eliperelman left a comment

Choose a reason for hiding this comment

helfi92 commented Nov 29, 2017 •

edited

Loading