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

feat: add version export #840

Merged
merged 2 commits into from
Jun 6, 2019
Merged

feat: add version export #840

merged 2 commits into from
Jun 6, 2019

Conversation

montezume
Copy link
Contributor

Summary

Like similar PRs in App Kit, we also add a version export 😄

@montezume montezume requested review from emmenko and tdeekens and removed request for emmenko June 6, 2019 08:48
@montezume montezume self-assigned this Jun 6, 2019
@@ -18,6 +18,9 @@ const configureRollupPlugins = (options = {}) =>
[
replace({
'process.env.NODE_ENV': JSON.stringify('production'),
'process.env.npm_package_version': JSON.stringify(
process.env.npm_package_version
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. If this works I'll simplify app-kit again too again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we don't use lerna here for bumping versions... I think that's why we can have this simpler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2019-06-06 at 10 51 42 AM

screenshot of cjs file

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice.

@montezume montezume requested a review from emmenko June 6, 2019 08:53
@@ -9,6 +9,12 @@ describe('exports', () => {
expect(customProperties).toBeTruthy();
});

describe('version', () => {
it('should match package.json version', () => {
expect(version).toMatch(process.env.npm_package_version);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bundle tests 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or should I directly import from package.json 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I think either is fine.

@@ -9,6 +9,12 @@ describe('exports', () => {
expect(customProperties).toBeTruthy();
});

describe('version', () => {
it('should match package.json version', () => {
expect(version).toMatch(process.env.npm_package_version);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think either is fine.

@montezume montezume requested a review from jonnybel June 6, 2019 09:37
Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

Nice! Yeah, here we can simply do it in rollup, as the release works differently.

@montezume montezume merged commit ab98dbc into master Jun 6, 2019
@adnasa adnasa deleted the ml-add-version branch April 9, 2021 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants