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: sideEffects: false cause css file not bundled (fixes #10190) #10197

Merged
merged 1 commit into from Apr 23, 2018

Conversation

Projects
None yet
4 participants
@Aladdin-ADD
Contributor

Aladdin-ADD commented Apr 23, 2018

First of all, thank you for your contribution! :-)

Please makes sure that these checkboxes are checked before submitting your PR, thank you!

  • Make sure that you propose PR to right branch: bugfix for master, feature for latest active branch feature-x.x.
  • Make sure that you follow antd's code convention.
  • Run npm run lint and fix those errors before submitting in order to keep consistent code style.
  • Rebase before creating a PR to keep commit history clear.
  • Add some descriptions and refer relative issues for you PR.

Extra checklist:

if isBugFix :

  • Make sure that you add at least one unit test for the bug which you had fixed.

elif isNewFeature :

  • Update API docs for the component.
  • Update/Add demo to demonstrate new feature.
  • Update TypeScript definition for the component.
  • Add unit tests for the feature.
@yutingzhao1991

This comment has been minimized.

Contributor

yutingzhao1991 commented Apr 23, 2018

close #10190.

@ant-design-bot

This comment has been minimized.

ant-design-bot commented Apr 23, 2018

Deploy preview for ant-design ready!

Built with commit 36ae0de

https://deploy-preview-10197--ant-design.netlify.com

@@ -212,5 +212,7 @@
"pre-commit": [
"lint-staged"
],
"sideEffects": false
"sideEffects": [
"es/**/style/*"

This comment has been minimized.

@whtsky

whtsky Apr 23, 2018

Contributor

what about styles under lib/ and dist/?

This comment has been minimized.

@yutingzhao1991

yutingzhao1991 Apr 23, 2018

Contributor

This problem only exist when use tree shaking with es6 module, so not need to include lib ant dist.

This comment has been minimized.

@whtsky

whtsky Apr 24, 2018

Contributor

babel-import-plugin imports from lib folder so there's still no css bundled in 3.4.3

This comment has been minimized.

@yutingzhao1991

yutingzhao1991 Apr 24, 2018

Contributor

let me see, @whtsky do you have any repo for test.

This comment has been minimized.

@whtsky

whtsky Apr 24, 2018

Contributor

I don't have any public repo for this, but I can confirm there's still no css in our project with
['import', { libraryName: 'antd', style: true }] config.

I have already opened #10217 for this

@codecov

This comment has been minimized.

codecov bot commented Apr 23, 2018

Codecov Report

Merging #10197 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #10197   +/-   ##
=======================================
  Coverage   86.34%   86.34%           
=======================================
  Files         197      197           
  Lines        4745     4745           
  Branches     1324     1324           
=======================================
  Hits         4097     4097           
  Misses        645      645           
  Partials        3        3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14fa376...36ae0de. Read the comment docs.

@yutingzhao1991 yutingzhao1991 merged commit 14dbcb9 into ant-design:master Apr 23, 2018

5 checks passed

codecov/patch Coverage not affected when comparing 6a3e6f5...36ae0de
Details
codecov/project 86.34% remains the same compared to 6a3e6f5
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
security/snyk No new issues
Details

@Aladdin-ADD Aladdin-ADD deleted the Aladdin-ADD:patch-1 branch Apr 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment