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

feat!: make package an esModule, upgrade deps, update build path for cjs #340

Merged
merged 6 commits into from
Apr 28, 2022
Merged

feat!: make package an esModule, upgrade deps, update build path for cjs #340

merged 6 commits into from
Apr 28, 2022

Conversation

seedy
Copy link
Contributor

@seedy seedy commented Apr 6, 2022 •
edited
Loading

Description

Transforms the project into an esmodule, forcing commonJs code to be named .cjs

Main export is now esModule rather than commonJs.

Dependency changes

Dependency Version State
React 17.0.1 -> 17.0.2 Updated
Storybook 6.3.7 -> 6.4.22 Updated
  • storybook, which was run with npx sb upgrade. It is mandatory to be able to run the storybook from within an esmodule project
  • react update was done only to align with latest version before 18, it also helped testing integration with yarn link

Known bugs

Storybook dark mode dependency no longer applies dark mode to "doc" pages

Breaking changes

  • project exports changed
  • project is now an esmodule, commonJs files must hold the .cjs extension
  • no longer needed to transpile the library in NextJs
  • NextJs will now consider the project an esmodule (previously commonJs)

How to test

  • run npx @skypack/package-check, check everything is valid
  • review changes
  • Try to run the project from another one using Faency, ex: https://github.com/seedy/test-faency
  • Check bundle size didn't grow

Good PR checkboxes

  • Change has been tested
  • Added/Updated tests
  • Added/Updated stories
  • PR follows conventions
  • Labels are set
  • Project is linked

Good Review checkboxes

ℹ️ Copy the snippet and paste in the review field to fill it 
- [ ] I've tested the changes
- [ ] I've agreed on the unit tests (soon to come)
- [ ] I've checked the stories
- [ ] I've read the code and understood it
- [ ] I don't have any more questions
- [ ] I've described any optional improvements
- [ ] I checked PR follows [conventions](https://github.com/traefik/faency#how-to-contribute)

@seedy seedy changed the title chore!: improve package quality chore: improve package quality Apr 6, 2022
@seedy seedy force-pushed the chore/improve-package-quality branch from fe57dfb to 08d3d39 Compare April 6, 2022 12:54
matthieuh
matthieuh reviewed Apr 11, 2022
View reviewed changes
Copy link
Contributor

@matthieuh matthieuh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would suggest checking first with the OSS team first as a license being set is immutable. CC @ldez

@seedy seedy requested a review from ldez April 11, 2022 09:19
@seedy seedy changed the title chore: improve package quality chore: improve package quality, add license Apr 11, 2022
@seedy seedy force-pushed the chore/improve-package-quality branch from 08d3d39 to 9accd43 Compare April 15, 2022 09:01
@seedy seedy force-pushed the chore/improve-package-quality branch from 9accd43 to 1486333 Compare April 25, 2022 09:23
@seedy seedy self-assigned this Apr 25, 2022
@seedy seedy marked this pull request as draft April 25, 2022 15:26
@seedy seedy removed request for ldez and gndz07 April 26, 2022 09:01
@seedy seedy force-pushed the chore/improve-package-quality branch from 6da5978 to d16fb9e Compare April 26, 2022 13:48
@seedy seedy changed the title chore: improve package quality, add license chore: improve package quality Apr 26, 2022
@seedy seedy changed the title chore: improve package quality fix!: properly bundle package, improve quality Apr 27, 2022
@seedy seedy changed the title fix!: properly bundle package, improve quality fix!: properly bundle package, improve bundle compatibility Apr 27, 2022
@seedy seedy changed the title fix!: properly bundle package, improve bundle compatibility feat!: properly bundle package, improve bundle compatibility Apr 27, 2022
@seedy seedy marked this pull request as ready for review April 27, 2022 10:26
@seedy seedy removed their assignment Apr 27, 2022
@seedy seedy added the kind/bug/fix a bug fix label Apr 27, 2022
paulocfjunior
paulocfjunior approved these changes Apr 27, 2022
View reviewed changes
Copy link
Contributor

@paulocfjunior paulocfjunior left a comment

Choose a reason for hiding this comment

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

LGTM

As the PR title will become the title of the commit after the merge, I'd suggest rewording it to something like "feat!: make the project an ES Module, upgrade deps". WDYT?

seedy
seedy commented Apr 28, 2022
View reviewed changes
rollup.config.js Show resolved Hide resolved
matthieuh
matthieuh approved these changes Apr 28, 2022
View reviewed changes
Copy link
Contributor

@matthieuh matthieuh left a comment

Choose a reason for hiding this comment

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

LGTM 👌

@seedy seedy changed the title feat!: properly bundle package, improve bundle compatibility feat!: make package an esModule, upgrade deps, fix build path for cjs Apr 28, 2022
@seedy seedy changed the title feat!: make package an esModule, upgrade deps, fix build path for cjs feat!: make package an esModule, upgrade deps, update build path for cjs Apr 28, 2022
@traefiker traefiker force-pushed the chore/improve-package-quality branch from d16fb9e to 93bebfb Compare April 28, 2022 15:06
@traefiker traefiker merged commit 2a58366 into traefik:master Apr 28, 2022
@traefiker
Copy link
Contributor

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@paulocfjunior paulocfjunior paulocfjunior approved these changes

@matthieuh matthieuh matthieuh approved these changes

@gndz07 gndz07 Awaiting requested review from gndz07

Assignees
No one assigned
Labels
kind/bug/fix a bug fix released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

V4 release is not compatible with jest tests Improve package quality with package-check
4 participants
@seedy @traefiker @matthieuh @paulocfjunior