Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

fix rollup imports and sourcemap support(jest + rollup) #489

Merged
merged 6 commits into from
Feb 20, 2018

Conversation

jbaxleyiii
Copy link
Contributor

Fixes #248

cc @evans

@evans evans changed the title fix rollup imports fix rollup imports and sourcemap support(jest + rollup) Feb 18, 2018
@evans
Copy link
Contributor

evans commented Feb 18, 2018

@jbaxleyiii I added sourcemap support for jest coverage and rollup builds(wasn't picking up the tsc sourcemaps). Tested with example from #248, so should work 🙏. We should definitely think about moving to RxJs if they have addressed the bundle size issues

@apollo-cla
Copy link

apollo-cla commented Feb 18, 2018

Messages
📖

Please add your name and email to the AUTHORS file (optional)

📖

If this was a change that affects the external API, please update the docs and post a link to the PR in the discussion

Generated by 🚫 dangerJS

@codecov-io
Copy link

codecov-io commented Feb 18, 2018

Codecov Report

Merging #489 into master will increase coverage by 4.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #489      +/-   ##
==========================================
+ Coverage    91.3%   95.67%   +4.36%     
==========================================
  Files          19       18       -1     
  Lines         828      601     -227     
  Branches      204      146      -58     
==========================================
- Hits          756      575     -181     
+ Misses         67       20      -47     
- Partials        5        6       +1
Impacted Files Coverage Δ
packages/apollo-link/src/link.ts 90.19% <ø> (-2.54%) ⬇️
packages/apollo-link/src/test-utils/mockLink.ts 85.71% <ø> (+0.71%) ⬆️
packages/apollo-link/src/linkUtils.ts 97.67% <ø> (+8.63%) ⬆️
packages/apollo-link/src/test-utils/setContext.ts 87.5% <ø> (-2.98%) ⬇️
...ackages/apollo-link/src/test-utils/testingUtils.ts 100% <0%> (ø) ⬆️
packages/apollo-link-retry/src/retryFunction.ts 100% <0%> (ø) ⬆️
packages/apollo-link-retry/src/delayFunction.ts 100% <0%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fcfa9c...6fdb63d. Read the comment docs.

@evans
Copy link
Contributor

evans commented Feb 19, 2018

apollo-link's bundle now emits Observable = Observable && Observable.hasOwnProperty('default') ? Observable['default'] : Observable; and the index contains import Observable from 'zen-observable'; , so we should be set

@stefanoTron
Copy link

awesome! when will this be merged? 😊 thanks!

@jbaxleyiii jbaxleyiii merged commit 98f9d15 into master Feb 20, 2018
@jbaxleyiii
Copy link
Contributor Author

@evans AMAZING

@jbaxleyiii jbaxleyiii deleted the rollup-fix branch February 20, 2018 17:57
@evans
Copy link
Contributor

evans commented Feb 20, 2018

@littletower We merged and published. Can you post in #389 if it still doesn't work with 1.2.0

@JounQin
Copy link

JounQin commented Feb 21, 2018

It breaks #511, and requires
adding allowSyntheticDefaultImports: true into tsconfig.json, is that reasonable to require specific tsconfig?

@evans
Copy link
Contributor

evans commented Feb 21, 2018

@JounQin Good point! We shouldn't be forcing a specific tsconfig.json. I think the best way forward is to either switch to RxJs or write an implementation of zen-observable in typescript, since the * as imports break js builds and a direct import necessitates the allowSyntheticDefaultImports. Please take a look at #515. If you think there is a better way forward please share.

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.

6 participants