Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

Export internals #149

Merged
merged 1 commit into from
Dec 3, 2020
Merged

Export internals #149

merged 1 commit into from
Dec 3, 2020

Conversation

samijaber
Copy link
Contributor

Overview

@github-actions
Copy link

github-actions bot commented Dec 2, 2020

size-limit report 📦

Path Size
dist/unsplash-js.cjs.production.min.js 3.82 KB (+7.3% 🔺)
dist/unsplash-js.esm.js 3.83 KB (+7.08% 🔺)

@samijaber samijaber marked this pull request as ready for review December 2, 2020 19:09
@samijaber samijaber requested review from a team and Magellol and removed request for a team December 2, 2020 19:09
@Magellol
Copy link
Member

Magellol commented Dec 2, 2020

I'm guessing these are things we'll need in unsplash's codebase? We could potentially make a second entrypoint for them.

import * as UnsplashInternals from 'unsplash-js/internals'

WDYT?

@samijaber
Copy link
Contributor Author

samijaber commented Dec 3, 2020

I'm guessing these are things we'll need in unsplash's codebase? We could potentially make a second entrypoint for them.

import * as UnsplashInternals from 'unsplash-js/internals'

WDYT?

I gave that a shot but bumped into some issues getting it right. It's hard to iron out what's happening because I'm using tsdx which is wrapping around the rollup config and making it less transparent. Very quickly hit the point where tsdx is more harm than good lol. They also have a short-sighted mentality around not providing a tsdx eject option. 🙄

I want to unblock the PR in the other repo, so I'll add this as a nice-to-have and we can always go back and improve the import later.

EDIT: I think I found my issue: it is a bug in tsdx jaredpalmer/tsdx#175 😒

Copy link
Member

@Magellol Magellol left a comment

Choose a reason for hiding this comment

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

Agreed let's not wait for it. We can revisit this whenever this issue is resolved. Might be a good idea to track it on our side.

@samijaber samijaber merged commit bf1266d into master Dec 3, 2020
@samijaber samijaber deleted the maintenance/expose-internals branch December 3, 2020 17:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants