Skip to content
This repository was archived by the owner on Dec 22, 2020. It is now read-only.

Conversation

@alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented May 15, 2019

BREAKING CHANGE: loader don't generate comment by default

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Don't generate comment by default because it can break not js assets (like css, html and etc). Better avoid any code generation inside loader.

Breaking Changes

Yes

Additional Info

No

@codecov
Copy link

codecov bot commented May 15, 2019

Codecov Report

Merging #24 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #24   +/-   ##
=======================================
  Coverage   77.77%   77.77%           
=======================================
  Files           2        2           
  Lines           9        9           
  Branches        1        1           
=======================================
  Hits            7        7           
  Misses          2        2
Impacted Files Coverage Δ
src/index.js 66.66% <50%> (ø) ⬆️

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 779a2f5...71e7c0e. Read the comment docs.

@alexander-akait alexander-akait force-pushed the fix-handle-not-javascript-modules branch from 05ab483 to 9d988d3 Compare May 15, 2019 11:20
@endiliey
Copy link

Thanks. This PR should close out #21 and #20

@alexander-akait
Copy link
Member Author

@endiliey in theory we can add option data to allow generate any custom information (like /* It was removed because it is polyfill */ - using { test: path.resolve(__dirname, 'polyfill.js'), loader: 'null-loader', options: { data: '/* It was removed because it is polyfill */'} }), but i thinks it is no need, anyway if somebody need this - PR welcome.

BREAKING CHANGE: loader don't generate comment by default
@alexander-akait alexander-akait force-pushed the fix-handle-not-javascript-modules branch from 9d988d3 to 71e7c0e Compare May 15, 2019 12:12
@alexander-akait alexander-akait merged commit 9dcd860 into master May 15, 2019
@alexander-akait alexander-akait deleted the fix-handle-not-javascript-modules branch May 15, 2019 13:10
@tubalmartin
Copy link

Thank you fore releasing a better yet solution and sorry for not committing the tests in #20 i've been really busy these past days.

@alexander-akait
Copy link
Member Author

@tubalmartin don't worry, sometimes i seem a little slow but i have a lot of issues and PRs

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.

4 participants