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: better tree shaking with @carbon/icons-react and carbon-react-components #209

Merged
merged 6 commits into from
Sep 23, 2021

Conversation

jhpedemonte
Copy link
Member

@jhpedemonte jhpedemonte commented Sep 3, 2021

What do these changes do/fix?

Slightly updated version of #208.

Fixes #202

How do you test/verify these changes?

Compare building the example app with and without these changes:

Building from master:
  874.61 KB    build/static/js/2.45b3be80.chunk.js
  25.52 KB     build/static/css/main.0f5e9c85.chunk.css
  1.93 KB      build/static/js/main.97b18a0f.chunk.js
  785 B        build/static/js/runtime-main.6c9e6b2e.js
Building with this PR:
  508.65 KB (-365.96 KB)  build/static/js/2.523141c6.chunk.js
  25.52 KB                build/static/css/main.0f5e9c85.chunk.css
  1.92 KB (-7 B)          build/static/js/main.d87ceb41.chunk.js
  785 B                   build/static/js/runtime-main.6c9e6b2e.js

Have you documented your changes (if necessary)?

Are there any breaking changes included in this pull request?

@jhpedemonte
Copy link
Member Author

Build is failing during tests:

discovery-search-app: yarn run v1.22.11
discovery-search-app: $ yarn test:unit && yarn test:e2e
discovery-search-app: $ cross-env SKIP_PREFLIGHT_CHECK=true CI=1 react-scripts test --env=jsdom
discovery-search-app: FAIL src/__tests__/App.test.js
discovery-search-app:   ● Test suite failed to run
discovery-search-app:     /home/runner/work/discovery-components/discovery-components/node_modules/carbon-components-react/es/components/ListBox/index.js:7
discovery-search-app:     import * as _PropTypes from './ListBoxPropTypes';
discovery-search-app:     ^^^^^^
discovery-search-app:     SyntaxError: Cannot use import statement outside a module
discovery-search-app:        9 | var carbonComponents = require('carbon-components');
discovery-search-app:       10 | var carbonComponentsReact = require('carbon-components-react');
discovery-search-app:     > 11 | var ListBox = _interopDefault(require('carbon-components-react/es/components/ListBox'));
discovery-search-app:          |                               ^
discovery-search-app:       12 | var reactDom = require('react-dom');
discovery-search-app:       13 | var reactDom__default = _interopDefault(reactDom);
discovery-search-app:       14 |
discovery-search-app:       at ScriptTransformer._transformAndBuildScript (../../node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
discovery-search-app:       at ScriptTransformer.transform (../../node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
discovery-search-app:       at Object.<anonymous> (../../packages/discovery-react-components/dist/index.js:11:31)
discovery-search-app: Test Suites: 1 failed, 1 total

But when I run locally, it succeeds. Haven't been able to figure out why it's failing.

…ee_shaking

* origin/master:
  chore: publish v1.4.0-beta.7 [ci skip]
  fix: escape/unescape field name in search filters (#212)
  chore: publish v1.4.0-beta.6 [ci skip]
  fix: safari regex lookbehind (#211)
  chore: publish v1.4.0-beta.5 [ci skip]
  fix: escape field names before building aggregation string (#207)
@jhpedemonte
Copy link
Member Author

Fixed with guidance from facebook/create-react-app#9938 (comment)

@jhpedemonte jhpedemonte merged commit 0c3d14e into master Sep 23, 2021
@jhpedemonte jhpedemonte deleted the psulkava-improve_tree_shaking branch September 23, 2021 18:09
jhpedemonte added a commit that referenced this pull request Sep 23, 2021
* origin/master:
  chore: publish v1.4.0-beta.8 [ci skip]
  fix: better tree shaking with @carbon/icons-react and carbon-react-components (#209)
jhpedemonte added a commit that referenced this pull request Sep 23, 2021
…yarn/pdfjs-dist-2.9.359

* origin/master:
  chore(deps): bump debounce from 1.2.0 to 1.2.1 (#206)
  chore: publish v1.4.0-beta.8 [ci skip]
  fix: better tree shaking with @carbon/icons-react and carbon-react-components (#209)
  chore: publish v1.4.0-beta.7 [ci skip]
  fix: escape/unescape field name in search filters (#212)
  chore: publish v1.4.0-beta.6 [ci skip]
  fix: safari regex lookbehind (#211)
  chore: publish v1.4.0-beta.5 [ci skip]
  fix: escape field names before building aggregation string (#207)
  chore(deps-dev): bump audit-ci from 3.1.1 to 4.1.0 (#205)
  chore(deps-dev): bump eslint-plugin-import from 2.22.1 to 2.24.2 (#204)
  chore(deps-dev): bump @types/seedrandom from 2.4.28 to 3.0.1 (#197)
  chore(deps-dev): bump @rollup/plugin-alias from 3.1.1 to 3.1.5 (#201)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

discovery-react-components includes all of the @carbon/icons-react package rather than tree-shaking
3 participants