Skip to content

Add prehtml-loader to documentation #211

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

Closed
wants to merge 1 commit into from
Closed

Add prehtml-loader to documentation #211

wants to merge 1 commit into from

Conversation

denis-migdal
Copy link

See #206

This PR contains a:

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

Motivation / Use-Case

Cite new loader prehtml-loader in the documentation.

See #206

Breaking Changes

No breaking changes.

Additional Info

See #206

@jsf-clabot
Copy link

jsf-clabot commented Jan 13, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jan 13, 2020

Codecov Report

Merging #211 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #211   +/-   ##
=======================================
  Coverage   95.28%   95.28%           
=======================================
  Files           5        5           
  Lines         106      106           
  Branches       21       21           
=======================================
  Hits          101      101           
  Misses          4        4           
  Partials        1        1

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 1bb6bd0...6686865. Read the comment docs.

@alexander-akait
Copy link
Member

We remove interpolate flag for the next version, I will provide simple templting in examples, anyway for new loaders/plugins/other tools best place is https://github.com/webpack-contrib/awesome-webpack

Thanks for the PR

@denis-migdal
Copy link
Author

Mmmm… I am not a big fan of the new version, it breaks the retro-compatibility and is quite verbose (?) for the configuration for the webpack.config.js.

In my opinion, it would have been better to have a js file to perform the preprocessing like I have done in my module, with a separate JS file executed at compilation time, able to modify directly the DOM with JQuery. This prevents the webpack.config.js to be way too verbose.

I will downgrade my version of html-loader to keep using interpolate.

@denis-migdal
Copy link
Author

But maybe should we speak about it ?

@alexander-akait
Copy link
Member

@denis-migdal We add the example in README how to migrate from interpolation

breaks the retro-compatibility

Yes because it is breaking change

I will downgrade my version of html-loader to keep using interpolate.

I don't recommend, html-loader@old doesn't work correction with webpack and other loaders, so you can get broken a code and a broken app

Whu do you need interpolation, it was always bad idea and broken in many cases

@denis-migdal
Copy link
Author

denis-migdal commented Mar 31, 2020

@denis-migdal We add the example in README how to migrate from interpolation

Yeah, but it would requires me to modify my existing HTML files (never a good idea in bigs projects), it is verbose in the webpack.config.js, and adds some features that, in my opinion, I handle better.

I will downgrade my version of html-loader to keep using interpolate.

I don't recommend, html-loader@old doesn't work correction with webpack and other loaders, so you can get broken a code and a broken app

It works correctly for me. There were indeed some bugs because some loader started using ES6, but nothing I can't fix.

There were a bug due to minimization, because you performed the minification before the interpolation (which was quite a strange thing to do in my opinion). I just disabled the minification and used html-minifier-loader.

Whu do you need interpolation, it was always bad idea and broken in many cases

It is not a bad idea, to the contrary. I use it to build HTML components I can include in other HTML page, or to build HTML templates (all at compilation time). And it works quite well.

But maybe I should stop using html-loader and reimplements the require() and interpolation features.

Note: I did not update the documentation for prehtml-loader, I will try to do it in few days to show you what I mean. Or I can demo it to you if you want, e.g. throw Skype.

@alexander-akait
Copy link
Member

Yeah, but it would requires me to modify my existing HTML files (never a good idea in bigs projects), it is verbose in the webpack.config.js, and adds some features that, in my opinion, I handle better.

So it is breaking change, some features can't be compatibility with new version, don't forget require is broken for old html-loader

Note: I did not update the documentation for prehtml-loader, I will try to do it in few days to show you what I mean. Or I can demo it to you if you want, e.g. throw Skype.

Please write here, it is allow to other developers read and find the best solution for their code base.

It is not a bad idea, to the contrary. I use it to build HTML components I can include in other HTML page, or to build HTML templates (all at compilation time). And it works quite well.

It is really bad idea, you try to implement HTML-in-JS/HTML-in-HTML (like import/require) in very dirty way. This idea in itself makes sense, but it should be organized on client side, not a build step, look on the future - <template> and <slot> tags (yes it not supported all browsers, but we have polyfills). I'm afraid you still have to rewrite/reorganize a project, so I would advise you to start doing it now. If required, I can give feedback on any question.

@denis-migdal
Copy link
Author

So it is breaking change, some features can't be compatibility with new version, don't forget require is broken for old html-loader

Seems to work quite well for me. After, I did some fixes in my module, e.g. by resolving paths.

Please write here, it is allow to other developers read and find the best solution for their code base.
I will give you a link when it will be done (maybe next week).

It is really bad idea, you try to implement HTML-in-JS/HTML-in-HTML (like import/require) in very dirty way. This idea in itself makes sense, but it should be organized on client side, not a build step, look on the future - <template> and <slot> tags (yes it not supported all browsers, but we have polyfills). I'm afraid you still have to rewrite/reorganize a project, so I would advise you to start doing it now. If required, I can give feedback on any question.

I strongly disagree, and I do not see why it would be dirty.

Why should it be done in the client-side ? Better doing it on the server-side (like we would have done in PHP), so that we send all required content at once.

template/slot seems quite interesting, and I could indeed use them in some cases in complement of what I am doing. Instead of making a "display:none" element and cloning it, I could just add an element.

But still, I am able to factorize HTML files, add JS in the main bundle depending on the components I load in my page, and use their API in the JS of my main page.

But maybe we are not speaking of the same things, I should do some quick example/documentation.

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.

3 participants