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.

Sign up for GitHub

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

Jump to bottom

fix: create umd build #772

Closed
wants to merge 7 commits into from
Closed

fix: create umd build #772

wants to merge 7 commits into from
+863 −35

Conversation

jbadan
Copy link
Contributor

@jbadan jbadan commented Oct 16, 2019 •
edited
Loading

Description

@jbadan jbadan requested a review from a team October 16, 2019 00:48
@netlify
Copy link

netlify bot commented Oct 16, 2019 •
edited
Loading

Deploy preview for fundamental-react ready!

Built with commit c6c6363

https://deploy-preview-772--fundamental-react.netlify.com

@jbadan jbadan changed the title fix: buildIndex script was exporting __ files fix: buildIndex script was exporting __ files + babel config edits Oct 16, 2019
@jbadan jbadan changed the title fix: buildIndex script was exporting __ files + babel config edits WIP: fix: buildIndex script was exporting __ files + babel config edits Oct 16, 2019
@jbadan jbadan changed the title WIP: fix: buildIndex script was exporting __ files + babel config edits WIP: fix: publishing improvements Oct 16, 2019
@markpalfreeman
Copy link
Contributor

markpalfreeman commented Oct 16, 2019 •
edited
Loading

I'm curious: do we need to include Rollup in addition to webpack? It seems like we may be creating more overhead just to publish a different lib type.

Can we do this with some changes to our current build config (or switch to Rollup for everything)? https://webpack.js.org/guides/author-libraries/#expose-the-library

@jbadan
Copy link
Contributor Author

jbadan commented Oct 17, 2019

@markpalfreeman That's a good idea - feel free to hop on this branch and try it out!

ChristianKienle
ChristianKienle reviewed Oct 22, 2019
View reviewed changes
scripts/rollup.config.js
@@ -0,0 +1,78 @@
import babel from 'rollup-plugin-babel';
Copy link

Choose a reason for hiding this comment

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

rollup <3

@jbadan jbadan changed the title WIP: fix: publishing improvements fix: publishing improvements, publish fix for 0.7.0 not publishing components + umd build Oct 24, 2019
@jbadan
Copy link
Contributor Author

jbadan commented Oct 24, 2019

@markpalfreeman let's stick with the standard rollup for now and iterate on trying out webpack later.

hertweckhr1
hertweckhr1 approved these changes Oct 24, 2019
View reviewed changes
Copy link
Contributor

@hertweckhr1 hertweckhr1 left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@markpalfreeman
Copy link
Contributor

I'm not trying to avoid this; the only reason I haven't approved this is because I just don't fully understand it 🤷‍♂

I'd feel more comfortable to merge the npm publish lib change and investigate the library files separately... but maybe we can discuss briefly as a team?

@jbadan jbadan mentioned this pull request Oct 25, 2019
Merged
@jbadan jbadan changed the title fix: publishing improvements, publish fix for 0.7.0 not publishing components + umd build fix: create umd build Oct 25, 2019
@jbadan jbadan deleted the fix/build-index-script branch February 26, 2020 00:31
@jbadan jbadan mentioned this pull request Apr 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristianKienle ChristianKienle ChristianKienle left review comments

@hertweckhr1 hertweckhr1 hertweckhr1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jbadan @markpalfreeman @ChristianKienle @hertweckhr1 @fundamental-bot