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

Prevented source maps from being injected into html #91

Merged
merged 2 commits into from
Dec 12, 2017
Merged

Prevented source maps from being injected into html #91

merged 2 commits into from
Dec 12, 2017

Conversation

WiseBird
Copy link

@WiseBird WiseBird commented Dec 11, 2017

Currently if source maps are configured for dll bundle they got injected into html:
2

As reproduction you can check https://github.com/WiseBird/auto_dll_source_maps/tree/source_maps_test (note this is a branch)

  • npm i
  • npm run start:dll

This PR filters them out by file name.

@sudo-suhas
Copy link
Collaborator

Is there such a thing as css sourcemaps?
Also, instead of blacklisting the file types, would it be better to whitelist them?

@WiseBird
Copy link
Author

@sudo-suhas Agree, its better to filter exactly for js when injecting. Also I added css files injection.

Copy link
Collaborator

@sudo-suhas sudo-suhas left a comment

Choose a reason for hiding this comment

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

LGTM.

@asfktz
Copy link
Owner

asfktz commented Dec 12, 2017

Hi @WiseBird,
Thanks for the PR!

instead of blacklisting the file types, would it be better to whitelist them?

I Agree too

Is there such a thing as css sourcemaps?

Yeah actually, there is.
For example, when you use CSS preprocessors like Sass or Less and you want to see the original filename & line number in devtools.

Merged 🎉

@asfktz asfktz merged commit cda780b into asfktz:master Dec 12, 2017
@WiseBird
Copy link
Author

Thank you guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants