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

added browser to package.json, resolves #1102 #1540

Merged
merged 1 commit into from
Aug 25, 2019

Conversation

ouijan
Copy link
Contributor

@ouijan ouijan commented Aug 22, 2019

Before creating a pull-request, please check https://github.com/wycats/handlebars.js/blob/master/CONTRIBUTING.md first.

Generally we like to see pull requests that

  • Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
  • Maintain the existing code style
  • Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
  • Have good commit messages
  • Have tests
  • Have the typings (lib/handlebars.d.ts) updated on every API change. If you need help, updating those, please mention that in the PR description.
  • Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
  • Currently, the 4.x-branch contains the latest version. Please target that branch in the PR.

@nknapp nknapp merged commit 9aaace7 into handlebars-lang:4.x Aug 25, 2019
@nknapp
Copy link
Collaborator

nknapp commented Aug 25, 2019

Released in 4.1.2-0 and 4.2.0

@nknapp
Copy link
Collaborator

nknapp commented Aug 25, 2019

I have released this change with a pre-release version so that people can test it before it becomes a real release.

@nknapp
Copy link
Collaborator

nknapp commented Aug 31, 2019

@ouijan have you been able to test this fix with your setup?

I am a bit irritated because @kpdecker gave it the milestone 5.0.0. This might indicate a breaking change which means somebody's build will break when I release it in the next patch version.

I actually think about delaying this until we have some proper integration tests set up for various build scenarios. That said: Could you provide a minimal setup that reflects the way you are using Handlebars? (package.json, webpack-config etc, something that will load, compile and run a simple template)

I am currently setting up a framework for that purpose in the main repository.

@ouijan
Copy link
Contributor Author

ouijan commented Sep 10, 2019

Hey @nknapp.
Sorry, I didn't mean to rush this into version 4's release cycle.

I set up these to example intellisense failing to resolve handlebars. I'm not familiar with stackblitz but in our codebase this error causes compilation to fail as well. I'll have a play and see if I can get this reflected in these stackblitz examples too.

@nknapp
Copy link
Collaborator

nknapp commented Sep 10, 2019

There are no typings included in version 4.0.x This may be the problem. Btw. This PR is included in 4.2.0, not 4.1.2, I don't know why generator-release has release 4.1.2-0 when I made the prerelease. But 4.1.2 is without the browser-properties.

@nknapp
Copy link
Collaborator

nknapp commented Sep 10, 2019

You can try to include an integration test in the integration-testing directory.
Just add a project and a "test.sh" to run the test.

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