search

unic / estatico

[DEPRECATED] Estático – Static site generator for frontend unicorns
Other
121 stars 18 forks source link

Clean up node_module imports #23

Closed caillou closed 7 years ago

caillou commented 7 years ago

The main property of a module's package.json tells you, what module will be imprted.

In the case of jQuery, the package.json reads:

{
  ...
  "main": "dist/jquery.js",
  ...
}

So the following lines are equivalent:

import $ from 'jquery'
import $ from 'jquery/dist/jquery'
backflip commented 7 years ago

Thanks a lot for this! Two suggestions above (one only partly related to the change).

backflip commented 7 years ago

@caillou, what's your opinion on the proposed changes?

caillou commented 7 years ago

@backflip I think, we should not do the handlebars alias. if you start treating some node packages differently than others, it just introduces confusion. But that is my opinion. Also, I do not intend to spend more time on this PR, I don't mind if you want to change it. I trust that you'll take the right decision. ❤️

backflip commented 7 years ago

@caillou, I just remembered the reason for this: If we want to use our handlebars helpers config universally (in Gulp and in the browser), we have to alias handlebars to handlebars/runtime since it already registers them and returns a handlebars instance.

It might make sense to just return helper methods instead of already registering them with a handlebars instance, however, we would have to change our architecture a bit for this (an approach as in https://github.com/unic/estatico/pull/17 would not work anymore). So I'll just re-add this alias after merging and figure it out another time.

backflip commented 7 years ago

@orioltf Let's merge and re-add it in a second PR.