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

fix issue #6 #7

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

fix issue #6 #7

wants to merge 2 commits into from

Conversation

ymaxli
Copy link

@ymaxli ymaxli commented Aug 8, 2014

为避免m[1] === 'js'将路径中有点号的情况过滤掉,于是把整个条件都去掉了。
因为在这个项目下spm test运行不起来,所以没试过测试用例,不过在自己的项目上做了简单的测试,应该没有问题。
对源码理解可能不够透彻>_<�不知道这样修改是否遗漏了一些情况

@afc163
Copy link
Member

afc163 commented Aug 8, 2014

这个修改略粗暴...

@ymaxli
Copy link
Author

ymaxli commented Aug 8, 2014

但没太看明白那句判断想要做什么

@afc163
Copy link
Member

afc163 commented Aug 8, 2014

只针对 JS 文件进行包裹,但是这样判断不严谨,只能排除掉常用的后缀名了。

在原来的基础上加了个条件判断是否是前端常见的文件类型,如果是的话就不处理。
这样就能有效避免没加js后缀但是路径中有点号被误解析成非JS文件的情况了。
现在`isNotJSFile`这个正则里只加了`.css`和`.html`,以后遇见前端里可能的文件类型再做添加
@ymaxli
Copy link
Author

ymaxli commented Aug 8, 2014

看看最新的commit呢@@

@afc163
Copy link
Member

afc163 commented Aug 8, 2014

我已经修改提交了,还是感谢 PR

@afc163
Copy link
Member

afc163 commented Aug 8, 2014

5eb4e54

@ymaxli
Copy link
Author

ymaxli commented Aug 8, 2014

好的的>_<

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.

2 participants