-
Notifications
You must be signed in to change notification settings - Fork 18
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
BREAKING CHANGE: Move dist/module
to lib
. Export Flow types
#21
base: main
Are you sure you want to change the base?
Conversation
2ca5277
to
372d548
Compare
@westeezy Why are we wanting to do this? Can you explain more the reason behind this change? |
Great work @westeezy! I agree we @mnicpt that we should document the details in the PR description to describe the benefits with this approach. I think the big win here is eliminating the need to import from Before: import { getUserAgent } from 'cross-domain-utils/src'; After: import { getUserAgent } from 'cross-domain-utils'; With this PR the above import statement still gets the Flow types as well as supports the existing tree-shaking strategy. This change is a pre-req to converting this repo to TypeScript while still supporting Flow types for other repos that are still using Flow. |
@mnicpt we are currently directly importing flow files when we grab files from @gregjopa for the direct import we will utilize package export maps. I have to test that feature more in webpack@4 so for now we are keeping the src folder for that reason and a later change will add the import map to enable edit: package exports not supported in webpack v4. womp womp. will need to import from lib or another folder TODO:
|
Yeah @mnicpt the plan is to allow importing from either |
Also I am thinking of adding a lint rule in the future to ensure See #23. Where we will move to krakenjs scopes so that we can more easily manage teams in npm as we onboard and offboard folks. |
372d548
to
672f609
Compare
672f609
to
5eff00e
Compare
Moving esm files to `lib/` folder gives us a path to stop importing from `src/` when importing files for treeshaking. We can migrate to importing from `lib/` and still have flow definitions at the ready Tasks Completed: - Remove module folder - Output transpiled code and flow defs to `lib/`
5eff00e
to
25b0c2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, @westeezy !
lib/