Skip to content

Commit

Permalink
refactor!: replace module.exports with ES module export (#1674)
Browse files Browse the repository at this point in the history
  • Loading branch information
sodatea committed May 12, 2020
1 parent 8c6eb5d commit 0b35f5c
Show file tree
Hide file tree
Showing 15 changed files with 1,620 additions and 919 deletions.
26 changes: 26 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
version: 2
jobs:
build:
docker:
- image: circleci/node:lts

working_directory: ~/repo

steps:
- checkout

- restore_cache:
keys:
- v1-dependencies-{{ checksum "yarn.lock" }}
- v1-dependencies- # used if checksum fails

- run: yarn install

- save_cache:
paths:
- node_modules
- ~/.cache/yarn
key: v1-dependencies-{{ checksum "yarn.lock" }}

- run: yarn lint
- run: yarn test
21 changes: 18 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
"scripts": {
"dev": "tsc --watch",
"build": "tsc",
"pretest": "tsc",
"test": "jest",
"dev-example": "webpack-dev-server --config example/webpack.config.js --inline --hot",
"build-example": "rm -rf example/dist && webpack --config example/webpack.config.js --env.prod",
"lint": "prettier --write --parser typescript \"src/**/*.ts\"",
"lint": "prettier --write --parser typescript \"{src,test}/**/*.ts\"",
"prepublishOnly": "tsc"
},
"gitHooks": {
Expand All @@ -30,6 +32,7 @@
]
},
"dependencies": {
"@types/mini-css-extract-plugin": "^0.9.1",

This comment has been minimized.

Copy link
@danyadev

danyadev May 12, 2020

why is this dependency here and not in devDependencies?

This comment has been minimized.

Copy link
@sodatea

sodatea May 13, 2020

Author Member

😂Thanks for spotting this! I think I forgot to add -D when installing it. Will fix it in the next release.

"chalk": "^3.0.0",
"hash-sum": "^2.0.0",
"loader-utils": "^1.2.3",
Expand All @@ -41,28 +44,40 @@
"@babel/preset-env": "^7.7.7",
"@types/estree": "^0.0.42",
"@types/hash-sum": "^1.0.0",
"@types/jest": "^25.2.1",
"@types/loader-utils": "^1.1.3",
"@types/webpack": "^4.41.0",
"@types/webpack-merge": "^4.1.5",
"@vue/compiler-sfc": "^3.0.0-beta.9",
"babel-loader": "^8.0.6",
"cache-loader": "^4.1.0",
"css-loader": "^3.3.2",
"file-loader": "^5.0.2",
"jest": "^24.9.0",
"jest": "^25.5.4",
"lint-staged": "^9.5.0",
"memfs": "^3.1.2",
"mini-css-extract-plugin": "^0.8.0",
"prettier": "^1.19.1",
"pug": "^2.0.4",
"pug-plain-loader": "^1.0.0",
"stylus": "^0.54.7",
"stylus-loader": "^3.0.2",
"ts-jest": "^24.2.0",
"ts-jest": "^25.5.1",
"typescript": "^3.7.3",
"url-loader": "^3.0.0",
"vue": "^3.0.0-beta.9",
"webpack": "^4.41.2",
"webpack-cli": "^3.3.10",
"webpack-dev-server": "^3.9.0",
"webpack-merge": "^4.2.2",
"yorkie": "^2.0.0"
},
"jest": {
"preset": "ts-jest",
"testEnvironment": "node",
"testPathIgnorePatterns": [
"<rootDir>/dist/",
"<rootDir>/node_modules/"
]
}
}
14 changes: 8 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ try {
)
}

import * as webpack from 'webpack'
import webpack from 'webpack'
import path from 'path'
import qs from 'querystring'
import hash from 'hash-sum'
Expand All @@ -25,7 +25,9 @@ import { genHotReloadCode } from './hotReload'
import { genCSSModulesCode } from './cssModules'
import { formatError } from './formatError'

const VueLoaderPlugin = require('./plugin')
import VueLoaderPlugin from './plugin'

export { VueLoaderPlugin }

export interface VueLoaderOptions {
transformAssetUrls?: SFCTemplateCompileOptions['transformAssetUrls']
Expand All @@ -38,7 +40,10 @@ export interface VueLoaderOptions {

let errorEmitted = false

const loader: webpack.loader.Loader = function(source: string) {
export default function loader(
this: webpack.loader.LoaderContext,
source: string
) {
const loaderContext = this

// check if plugin is installed
Expand Down Expand Up @@ -245,6 +250,3 @@ function attrsToQuery(attrs: SFCBlock['attrs'], langFallback?: string): string {
}
return query
}

;(loader as any).VueLoaderPlugin = VueLoaderPlugin
module.exports = loader
6 changes: 3 additions & 3 deletions src/pitcher.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as webpack from 'webpack'
import webpack from 'webpack'
import qs from 'querystring'
import loaderUtils from 'loader-utils'

Expand All @@ -22,8 +22,6 @@ const isNotPitcher = (l: Loader) => l.path !== __filename

const pitcher: webpack.loader.Loader = code => code

module.exports = pitcher

// This pitching loader is responsible for intercepting all vue block requests
// and transform it into appropriate requests.
pitcher.pitch = function() {
Expand Down Expand Up @@ -126,3 +124,5 @@ function shouldIgnoreCustomBlock(loaders: Loader[]) {
})
return actualLoaders.length === 0
}

export default pitcher
17 changes: 11 additions & 6 deletions src/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
const webpack = require('webpack')
let VueLoaderPlugin = null
import webpack from 'webpack'
declare class VueLoaderPlugin implements webpack.Plugin {
static NS: string
apply(compiler: webpack.Compiler): void
}

let Plugin: typeof VueLoaderPlugin

if (webpack.version && webpack.version[0] > 4) {
if (webpack.version && webpack.version[0] > '4') {

This comment has been minimized.

Copy link
@danyadev

danyadev May 12, 2020

why use a string instead of a number?

// webpack5 and upper
VueLoaderPlugin = require('./pluginWebpack5')
Plugin = require('./pluginWebpack5').default
} else {
// webpack4 and lower
VueLoaderPlugin = require('./pluginWebpack4')
Plugin = require('./pluginWebpack4').default
}

module.exports = VueLoaderPlugin
export default Plugin
4 changes: 2 additions & 2 deletions src/pluginWebpack4.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import qs from 'querystring'
import * as webpack from 'webpack'
import webpack from 'webpack'
import { VueLoaderOptions } from './'

const RuleSet = require('webpack/lib/RuleSet')
Expand Down Expand Up @@ -196,4 +196,4 @@ function cloneRuleForRenderFn(rule: webpack.RuleSetRule) {
return res
}

module.exports = VueLoaderPlugin
export default VueLoaderPlugin
6 changes: 3 additions & 3 deletions src/pluginWebpack5.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import qs from 'querystring'
import { VueLoaderOptions } from './'
import { RuleSetRule, Compiler } from 'webpack'
import { RuleSetRule, Compiler, Plugin } from 'webpack'

const id = 'vue-loader-plugin'
const NS = 'vue-loader'
Expand Down Expand Up @@ -77,7 +77,7 @@ const ruleSetCompiler = new RuleSetCompiler([
new UseEffectRulePlugin()
])

class VueLoaderPlugin {
class VueLoaderPlugin implements Plugin {
static NS = NS

apply(compiler: Compiler) {
Expand Down Expand Up @@ -302,4 +302,4 @@ function cloneRule(
return res
}

module.exports = VueLoaderPlugin
export default VueLoaderPlugin
2 changes: 1 addition & 1 deletion src/select.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as webpack from 'webpack'
import webpack from 'webpack'
import { SFCDescriptor } from '@vue/compiler-sfc'
import { ParsedUrlQuery } from 'querystring'

Expand Down
4 changes: 2 additions & 2 deletions src/stylePostLoader.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import qs from 'querystring'
import { compileStyle } from '@vue/compiler-sfc'
import * as webpack from 'webpack'
import webpack from 'webpack'

// This is a post loader that handles scoped CSS transforms.
// Injected right before css-loader by the global pitcher (../pitch.js)
Expand All @@ -23,4 +23,4 @@ const StylePostLoader: webpack.loader.Loader = function(source, inMap) {
}
}

module.exports = StylePostLoader
export default StylePostLoader
4 changes: 2 additions & 2 deletions src/templateLoader.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as webpack from 'webpack'
import webpack from 'webpack'
import qs from 'querystring'
import loaderUtils from 'loader-utils'
import { VueLoaderOptions } from './'
Expand Down Expand Up @@ -62,4 +62,4 @@ const TemplateLoader: webpack.loader.Loader = function(source, inMap) {
loaderContext.callback(null, code, map)
}

module.exports = TemplateLoader
export default TemplateLoader
5 changes: 5 additions & 0 deletions test/core.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { bundle } from './utils'

test('basic', async () => {
await bundle({ entry: 'basic.vue' })
})
19 changes: 19 additions & 0 deletions test/fixtures/basic.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<template>
<h2 class="red">{{msg}}</h2>
</template>

<script>
export default {
data () {
return {
msg: 'Hello from Component A!'
}
}
}
</script>

<style>
comp-a h2 {
color: #f00;
}
</style>
9 changes: 9 additions & 0 deletions test/fixtures/entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Component from '~target'
import * as exports from '~target'

if (typeof window !== 'undefined') {
window.module = Component
window.exports = exports
}

export default Component

5 comments on commit 0b35f5c

@jods4
Copy link

@jods4 jods4 commented on 0b35f5c May 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beta.2 has broken my Webpack build, specifically my PostCss loaders on styles.
There are only 2 commits compared to beta.1 and I've tried reverting the other commit (didn't fix my issue).
So I'm left thinking this commit introduced a big breaking change.

I can't see what would have, but this commit restored Yarn packages and caught quite a few major version bumps in the lot, so that might be the cause.

@sodatea
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jods4 Do you have a reproduction repo?

@jods4
Copy link

@jods4 jods4 commented on 0b35f5c May 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sodatea JS is such a 💩 environment to work in. 👿
I just spent hours on this and I still have no idea what's wrong. I can't share the real project and my attempts at repro have failed.

When looking at the error:

ERROR in ./ClientApp/Components/Forms/input.scss?vue&type=style&index=0&lang=css (./node_modules/css-loader/dist/cjs.js!./node_modules/style-loader/dist/cjs.js!./node_modules/css-loader/dist/cjs.js!./node_modules/postcss-loader/src??postcss!./ClientApp/Components/Forms/input.scss?vue&type=style&index=0&lang=css)
Module build failed (from ./node_modules/css-loader/dist/cjs.js):
CssSyntaxError

It's clear that something (prob. vue-loader) has incorrectly manipulated the chain of loaders.
If you look closely at the chain you can see this:

  • css-loader (Wrong, should not be here!)
  • style-loader
  • css-loader (correct)
  • postcss-loader
  • ./ClientApp/Components/Forms/input.scss?vue&type=style&index=0&lang=css (the target, a scss file that was @imported in .vue).

I still have no idea what's going on and as I said simple repros work.
All I can say for now is that I didn't have this problem on beta-1.

@jods4
Copy link

@jods4 jods4 commented on 0b35f5c May 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sodatea Found it!
It's unrelated to post-css, even plain css is broken.

What broke is when you load an external file in a vue component, like so:

<style src="./test.css">
</style>

@jods4
Copy link

@jods4 jods4 commented on 0b35f5c May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sodatea ok now that I know precisely how to reproduce the error, I've opened #1679 for tracking.

Please sign in to comment.