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

Add rollup & multitarget support #11

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

stevefan1999-personal
Copy link

@stevefan1999-personal stevefan1999-personal commented Jul 30, 2020

Now we that we used rollup we can pack this in webpack at a much faster speed
Fixes #10

@@ -21,7 +23,7 @@ function getCmdOptions (cmd: any) {
}

program
.version(require('../../package.json').version)
.version(version)
Copy link
Owner

@yoyo930021 yoyo930021 Aug 1, 2020

Choose a reason for hiding this comment

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

This version is wrong on release.
Because bundling is before upgrading package.json version.

tsconfig: 'tsconfig.compile.json'
})
const pluginJson = json()
const pluginCommonJs = commonjs()
Copy link
Owner

Choose a reason for hiding this comment

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

The pluginCommonJs is needed when any output.
Because it is converting CommonJS modules to ES6 for dependencies

file: 'dist/index.browser.mjs',
format: 'es'
},
plugins: [replace({ 'process.env.BROWSER': 'true' }), pluginJson, pluginTypeScript]
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can replace process.env.BROWSER to __BROWSER__.

Comment on lines 9 to +10
const isNode = typeof window === 'undefined'
if (!isNode) {
if (process.env.BROWSER || !isNode) {
Copy link
Owner

Choose a reason for hiding this comment

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

We don't need isNode.

@@ -1 +1,4 @@
declare module 'prettier-eslint'
declare module 'prettier-eslint' {
const ret: any
Copy link
Owner

Choose a reason for hiding this comment

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

I don't know why about this code?

if (isVueFile(fileFsPath)) {
const scriptContent = parseVueFile(options.vueTemplateCompiler, fileContent).script
if (scriptContent) {
log(`Readed Vue file: ${fileFsPath}`)
if (scriptContent?.content != null) {
Copy link
Owner

Choose a reason for hiding this comment

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

Does we need != null ?

@@ -0,0 +1,46 @@
import commonjs from '@rollup/plugin-commonjs'
import json from '@rollup/plugin-json'
import replace from '@rollup/plugin-replace'
Copy link
Owner

Choose a reason for hiding this comment

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

We need to add replace({ 'process.env.BROWSER': 'false' }) for outputs without browser.

@yoyo930021
Copy link
Owner

Thanks for contribution.
Maybe we can remove Parcel for demo.

@@ -9,7 +9,7 @@ export interface Vc2cOptions {
compatible: boolean
setupPropsKey: string
setupContextKey: string
typesciprt: typeof ts
typescript: typeof ts
Copy link
Owner

Choose a reason for hiding this comment

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

Thank for fixing typo.

@yoyo930021
Copy link
Owner

This project is use this commit message style.
Can you change commit messages?
It can help tool for generate change log and release versions.

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.

Make this project compatible with webpack
2 participants