Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: component generator & universal build #967

Merged
merged 39 commits into from
Apr 25, 2019
Merged

Conversation

yacheng
Copy link
Collaborator

@yacheng yacheng commented Mar 5, 2019

Before submitting a pull request, please make sure the following is done...

  1. Fork the repo and create your branch from master.
  2. If you've added code that should be tested, add tests!
  3. If you've changed APIs, update the documentation.
  4. Ensure the test suite passes (npm test).
  5. Make sure your code lints (npm run lint) - we've done our best to make sure these rules match our internal linting guidelines.

Copy link
Collaborator

@wssgcg1213 wssgcg1213 left a comment

Choose a reason for hiding this comment

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

some comments.

],
},
// JSON is not enabled by default in Webpack but both Node and Browserify
// allow it implicitly so we also enable it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since webpack >= v2.0.0, importing of JSON files will work by default

@@ -72,6 +72,10 @@ function getConfig(entry, output, moduleOptions, babelLoaderQuery, target, devto
};
}


babelOptions.presets[1][1].modules = 'commonjs';
babelOptions.plugins.push('add-module-exports');
Copy link
Collaborator

Choose a reason for hiding this comment

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

do not do this to modify a reference object.

*.log

.DS_Store
.idea/
Copy link
Collaborator

Choose a reason for hiding this comment

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

把.idea丢到ignore里其实不是一种好的做法, ide的配置可能包含一些编码习惯, 同一个项目的编码习惯在团队中互相共享有助于保持项目代码的一致性, 不过我们现在好像都是这么做的, 只是提出一下, 可以先不改

@wssgcg1213
Copy link
Collaborator

take a look in travis-ci.

</style>
</head>
<body>
<script src="https://g.alicdn.com/code/npm/web-rax-framework/0.6.4/dist/framework.web.min.js"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

应该使用 1.0 的工程

@@ -48,6 +48,7 @@ const paths = {
appSrc: resolveApp('src'),
appManifest: resolveApp('manifest.json'),
appNodeModules: resolveApp('node_modules'),
componentIndexJs: resolveModule(resolveApp, 'public/index'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

public/index 是 js 文件?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

组件 demo 入口文件

],
},
{
test: /\.(sfc|vue|html)$/,
Copy link
Collaborator

Choose a reason for hiding this comment

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

sfc相关的应该可以不需要了,参考最新的webapp的配置脚本

@@ -0,0 +1,53 @@
const type = process.env.TYPE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

babel文件应该是共享的,参考整体目录结构,不应该重复

"babel-plugin-add-module-exports": "1.0.0",
"eslint": "^5.10.0",
"eslint-plugin-import": "^2.14.0",
"eslint-plugin-react": "~7.11.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

这些包不应该加到scripts里的依赖,这些是项目生成后里的依赖

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这部分之前考虑是 rax-scripts 也承担 lint 的工作,目前不考虑 lint 我先去掉

@@ -32,6 +32,10 @@
"@babel/preset-flow": "7.0.0",
"@babel/preset-react": "7.0.0",
"@babel/runtime": "^7.2.0",
"babel-plugin-add-module-exports": "1.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个包考虑不要使用

@@ -8,4 +8,5 @@ program
.usage('<command> [options]')
.command('build', 'Build project in production mode')
.command('start', 'Start a web server in development mode (hot-reload and inline-module-source-map is enable default)')
.command('lint', 'Lint for component code')
Copy link
Collaborator

Choose a reason for hiding this comment

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

lint 回归到项目本身

@@ -0,0 +1,12 @@
#!/usr/bin/env node
Copy link
Collaborator

Choose a reason for hiding this comment

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

lint 功能暂时都不在 scripts 里

@@ -0,0 +1,23 @@
import {Component, createElement, PropTypes, render} from 'rax';
Copy link
Collaborator

Choose a reason for hiding this comment

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

PropTypes, render 没有用到,不应该import进来

import renderer from 'rax-test-renderer';
import Mod from '../';

describe('Mod', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

“ Mod” 可以优化下内容

import Mod from '../';

describe('Mod', () => {
it('test typeof Mod', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

“test typeof Mod” 表达需要优化下

@@ -48,6 +48,7 @@ const paths = {
appSrc: resolveApp('src'),
appManifest: resolveApp('manifest.json'),
appNodeModules: resolveApp('node_modules'),
componentDemoJs: resolveModule(resolveApp, 'public/index'),
Copy link
Collaborator

@yuanyan yuanyan Apr 24, 2019

Choose a reason for hiding this comment

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

demo js 是不是可以考虑放到 src/demo.js 或 src/demo/index.js 或 demo/index.js

<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1, minimum-scale=1, user-scalable=no">
<title>Rax Component Demo</title>
<script src="//g.alicdn.com/code/npm/web-rax-framework/0.6.5/dist/framework.web.js"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

1.0 里不需要这个cdn文件了

@@ -17,6 +17,14 @@ var entry = {};
entry[entryName] = entry[entryName + '.min'] = entry[entryName + '.factory'] = main;
var globalName = uppercamelcase(packageName);

babelOptions.presets.push([
Copy link
Collaborator

Choose a reason for hiding this comment

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

prod 下为什么需要重复配置这个?

@@ -0,0 +1,23 @@
// jest.config.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

jest 配置可以考虑不对 component 级别暴露

@@ -0,0 +1,213 @@
var cliObject = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

cliObject 变量命名可以优化下

@yacheng yacheng merged commit 54190f3 into master Apr 25, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat-universal-build branch April 25, 2019 10:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants