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

Sourcemaps for rollup bundles #3042

Merged
merged 4 commits into from
Mar 14, 2018
Merged

Sourcemaps for rollup bundles #3042

merged 4 commits into from
Mar 14, 2018

Conversation

evans
Copy link
Contributor

@evans evans commented Feb 19, 2018

The ts-jest package requires the coverageMap option to be enabled to get accurate coverage mapping back to the typescript files.

The rollup bundles' sourcemaps were pointing to the compiled javascript files, now they point a the ts files.

@evans evans changed the title Sourcemaps to jest coverage and rollup bundles Sourcemaps for jest coverage and rollup bundles Feb 19, 2018
@apollo-cla
Copy link

apollo-cla commented Feb 19, 2018

Messages
📖

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

Generated by 🚫 dangerJS

@fbartho
Copy link

fbartho commented Feb 23, 2018

@evans Oh man‼️ Would you mind fixing this for apollo-link-rest? -- apollographql/apollo-link-rest#76

I'd be thrilled to merge that in there, ASAP. It's been driving me nuts, and I have a PR where I got a red-flagged code-coverage report and I want to increase it, and I can't figure out what lines are wrong.

I'm frederic on the public Slack if you want to chat, or we have #apollo-link-rest

Copy link

@fbartho fbartho left a comment

Choose a reason for hiding this comment

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

I think I found a bug. It's possible I don't understand the code-flow. I haven't run this code yet, I'm just trying to port it into apollo-link-rest (apollographql/apollo-link-rest#76).

config.output = Object.assign(
{
file: 'lib/bundle.umd.js',
format: 'umd',
name,
Copy link

Choose a reason for hiding this comment

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

If you look at the structure of this diff, I think there's a bug.

It looks like you're embedding fields that were outside of config.output, into the output object.

Copy link

Choose a reason for hiding this comment

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

It looks like a newer version of rollup wants these things moved round and renamed:

The following options have been renamed — please update your config: entry -> input, name -> output.name, sourceMap -> output.sourcemap, exports -> output.exports

@evans evans changed the title Sourcemaps for jest coverage and rollup bundles Sourcemaps for rollup bundles Mar 12, 2018
@evans evans merged commit de6de2e into master Mar 14, 2018
@evans evans deleted the sourcemaps branch March 14, 2018 18:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
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.

3 participants