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

Add External contents to webpack #6494

Merged
merged 2 commits into from Oct 9, 2017
Merged

Add External contents to webpack #6494

merged 2 commits into from Oct 9, 2017

Conversation

sudheerj
Copy link
Contributor

@sudheerj sudheerj commented Oct 8, 2017

  • Please make sure the below checklist is followed for Pull Requests.

  • Travis tests are green

  • Tests are added where necessary

  • Documentation is added/updated where necessary

  • Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

@sudheerj sudheerj mentioned this pull request Oct 8, 2017
4 tasks
const to = `content/${targetFolder}/`;
const webpackDevPath = `${CLIENT_WEBPACK_DIR}/webpack.common.js`;
let assetBlock = '';
if(from || to) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt this be from && to

@deepu105
Copy link
Member

deepu105 commented Oct 8, 2017

You have lint fail, make sure to run npm run lint on your fork

> generator-jhipster@4.9.0 lint /home/travis/build/jhipster/generator-jhipster
> eslint .
/home/travis/build/jhipster/generator-jhipster/generators/generator-base.js
  1102:9  error  Expected space(s) after "if"  keyword-spacing
✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

@deepu105
Copy link
Member

deepu105 commented Oct 8, 2017

dont create new PR just push your change commits into the same branch

@sudheerj
Copy link
Contributor Author

sudheerj commented Oct 8, 2017

Thanks Deepu. Done with the changes and no lint issues as well. By the way when we will have next Jhipster minor release. If it is not immediate, Do we have any nightly builds available. This is required for me to publish the module.

@deepu105
Copy link
Member

deepu105 commented Oct 9, 2017

I cannot give you an ETA on this and unfortunately, we don't do nightly builds. I suggest you use the master branch in the meantime.
Clone the repo, checkout master
run npm link on the cloned generator-jhipster folder, now this version will be used instead of global
cc @jdubois

@sudheerj
Copy link
Contributor Author

sudheerj commented Oct 9, 2017

Thanks. Currently I'am using local version of JHipster(with above changes) to work with PrimeNG module. But it cannot be published until JHipster released with above changes. If anybody tries to use the module then they are going to face below issues. i.e, Even users has to maintain the local version of Jhipster.

sudheerj/generator-jhipster-primeng#4

I will also raise two more PRs to inject third party resources(one for index.html and other one for Vendor.SCSS).

@pascalgrimaud
Copy link
Member

pascalgrimaud commented Oct 9, 2017

Look at https://github.com/geraldhumphries/generator-jhipster-elasticsearch-reindexer/blob/master/generators/app/index.js#L204-L210
you can use a similar code, so your module will work with projects which were generated with a version older than 4.9.x

@sudheerj
Copy link
Contributor Author

sudheerj commented Oct 9, 2017

Thanks Pascal. Is it the code snippet similar to above solves the issue of publishing module or the code snippets to work with multiple versions of JHipster.

@pascalgrimaud
Copy link
Member

Let me explain a little bit:
By adding this check on this function, your code will test if the function exists in the generator-jhipster dependency of the project (generated by the user):

  • if yes (case 1),
    • the project has probably been generated with generator-jhipster 4.9.1+ (which is not yet released)
    • your module can use this function
    • it will add some code into webpack.common.js
  • if no (case 2),
    • there will be no error when the user uses your module, because you catch the error
    • some instructions will be displayed to the user, so he can add manually the code needed into webpack.common.js

By using this code snippet, you can publish your module now. All users who use your module, will be on the case 2. As soon as there is a new release of generator-jhipster, it should be in the case 1.

Hope I'm clear.

@sudheerj
Copy link
Contributor Author

sudheerj commented Oct 9, 2017

Thanks a lot Pascal for the detailed explanation. I will add this check to avoid this kind of errors.

@jdubois jdubois added this to the 4.10.0 milestone Oct 17, 2017
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.

None yet

4 participants