Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

add babel esm and umd exports #536

Merged
merged 5 commits into from
Aug 15, 2019
Merged

Conversation

redallen
Copy link
Contributor

@redallen redallen commented Aug 6, 2019

Fix openshift/console#2232 upstream.

"prebuild": "yarn test --ci && shx rm -rf dist",
"build": "NODE_ENV=production yarn build:js && yarn build:sass",
"build:js": "babel src --config-file ./config/babel.config.js --out-dir dist/js --only 'src/**/*.js' --ignore 'src/jest,src/cosmos,**/tests/**,**/fixtures/**'",
"build": "yarn build:babel && yarn build:sass",
Copy link
Contributor

Choose a reason for hiding this comment

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

should NODE_ENV=production and && shx rm -rf dist be preserved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NODE_ENV=production doesn't matter. I added back the prebuild step, though.

@priley86
Copy link
Contributor

priley86 commented Aug 6, 2019

cc: @suomiy @mareklibra - it seems you'll need to export ESM to treeshake Console UI bundle. Thanks @redallen !

Copy link
Contributor

@atiratree atiratree left a comment

Choose a reason for hiding this comment

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

also, the tests are failing with

babel-jest: Babel ignores src/jest/setupTest.js - make sure to include the file in Jest's transformIgnorePatterns as well

package.json Outdated Show resolved Hide resolved
config/eslint.browser.js Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Aug 8, 2019

Pull Request Test Coverage Report for Build 1898

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 82.844%

Totals Coverage Status
Change from base Build 1888: 0.2%
Covered Lines: 4162
Relevant Lines: 4811

💛 - Coveralls

@redallen
Copy link
Contributor Author

redallen commented Aug 8, 2019

@suomiy I fixed the tests and addressed your feedback. We needed a CJS babel config for Jest to run the tests on Node, so I passed it manually to Jest.

CreateVMWizard.test.js seems to have something weird going on with it:

console.error node_modules/react/cjs/react.development.js:225
Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

Check the render method of `WizardPatternBody`.
    in WizardPatternBody (created by WizardPattern)
    in div (created by ModalBody)
    in ModalBody (created by WizardBody)
    in WizardBody (created by WizardPattern)
    in div (created by Wizard)

but I think that's outside the scope of this PR and is probably a PatternFly problem.

package.json Outdated Show resolved Hide resolved
@@ -59,5 +59,6 @@ module.exports = {
groups: [['builtin', 'external'], ['internal', 'parent', 'sibling', 'index']],
},
],
'import/no-cycle': 'off',
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove the dependency cycles in a follow-up.
Brief look - this should not be so difficult.

@redallen
Copy link
Contributor Author

@mareklibra Added the ignores.

@mareklibra
Copy link
Contributor

Thank you for the patch!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants