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

Webpack upgrade #1416

Closed
wants to merge 5 commits into from
Closed

Webpack upgrade #1416

wants to merge 5 commits into from

Conversation

jishnu7
Copy link

@jishnu7 jishnu7 commented Jan 1, 2018

Pull request to upgrade webpack and babel to respective latest versions. All test cases are working.

There is an issue with coverage. Istanbul doesn't support ES6-Babel properly, we will have to use either babel-istanbul or isparta. Since master branch is using a custom fork of istanbul, I'm letting you decide on how to proceed with it.

Another way to handle is, running test cases on original source rather than the babel generated one. If we are going to do that, then we don't need grunt-babel, we will be able to use babel-loader with webpack to generate just the dist files we need, rather than exporting all transpiled sources.

Relates to: #1386 , #1399 ,

@nknapp
Copy link
Collaborator

nknapp commented Jan 1, 2018

That sounds great. remap-istanbul would be another option. I have used it in a TypeScript-project and it worked fine. It needs node 6 though, but that is another discussion.

In any case, could you point your PR to the 4.x-branch? That is the branch of currently released versions. There has not been a release of the master branch yet. I'll merge into the master from 4.x then.

4.x is not using the custom fork of istanbul, so babel-istanbul might work fine.

@jishnu7 jishnu7 changed the base branch from master to 4.x January 3, 2018 17:53
@jishnu7
Copy link
Author

jishnu7 commented Jan 3, 2018

Made necessary changes and target branch to v4.x.

Using babel-istanbul straight away cleared the coverage except branches, which is having 99.91% coverage. Only one branch is not covered, but the interface is not clearly pointing out which section of the file is causing the issue. I'm checking if we can do something about it.

@jishnu7
Copy link
Author

jishnu7 commented Jan 17, 2018

amd build seems to be failing because of dynamic import. master branch doesn't seems to have amd builds. Do you use amd build anywhere?

@nknapp
Copy link
Collaborator

nknapp commented Feb 9, 2018

I don't know how many users Handlebars has, but I think the pobability that someone uses amd-builds, is pretty high.

@nknapp
Copy link
Collaborator

nknapp commented Sep 20, 2019

I'll close this for now. Thanks for trying back then. I think, with some more integration tests we can do another attempt, but not now.

@nknapp nknapp closed this Sep 20, 2019
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.

2 participants