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

Fix blueprint templates folders. #15970

Merged
merged 1 commit into from
Aug 15, 2021
Merged

Conversation

mshima
Copy link
Member

@mshima mshima commented Aug 14, 2021


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

When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@mshima mshima merged commit 87a393d into jhipster:main Aug 15, 2021
@mshima mshima deleted the skip_ci-fix_blueprint branch August 15, 2021 00:06
@pascalgrimaud pascalgrimaud added this to the 7.2.0 milestone Sep 11, 2021
@avdev4j
Copy link
Contributor

avdev4j commented Oct 13, 2021

hi @mshima
I'm facing an issue with a blueprint that includes a subgenerator that directly extends the baseblueprint (yes this subgen doesn't exist in JHipster).

Since the 7.2.0 we can't use it anymore because of an error and I guess this modification is related to this.

This is my error

ERROR! Error parsing namespace Users:anthonyviard:projects:entando:entando-blueprint:entity-microfrontend
Error: Error parsing namespace Users:anthonyviard:projects:entando:entando-blueprint:entity-microfrontend
at Environment.requireNamespace (/private/tmp/entando/node_modules/yeoman-environment/lib/namespace.js:68:13)
at module.exports.jhipsterTemplatePath (/Users/anthonyviard/projects/entando/entando-blueprint/node_modules/generator-jhipster/generators/generator-base.js:162:67)
at new JHipsterBaseBlueprintGenerator (/Users/anthonyviard/projects/entando/entando-blueprint/node_modules/generator-jhipster/generators/generator-base-blueprint.js:55:47)
at new module.exports (/Users/anthonyviard/projects/entando/entando-blueprint/generators/entity-microfrontend/index.js:14:5)
at Environment.instantiate (/private/tmp/entando/node_modules/yeoman-environment/lib/environment.js:802:23)
at Environment.create (/private/tmp/entando/node_modules/yeoman-environment/lib/environment.js:774:17)
at module.exports.composeWith (/Users/anthonyviard/projects/entando/entando-blueprint/node_modules/generator-jhipster/node_modules/yeoman-generator/lib/index.js:1239:42)
at module.exports.composeMicrofrontend (/Users/anthonyviard/projects/entando/entando-blueprint/generators/entity/index.js:79:16)
at Object. (/Users/anthonyviard/projects/entando/entando-blueprint/node_modules/generator-jhipster/node_modules/yeoman-generator/lib/index.js:1024:25)
at /Users/anthonyviard/projects/entando/entando-blueprint/node_modules/run-async/index.js:49:25

I can see the following method jhipsterTemplatePath() is called and raise an error because of the namespace resolution this.env.requireNamespace(this.options.namespace).generator

I guess this code should be applied only on a "real" extended JHipster generator.

there is my sub generator: https://github.com/entando/entando-blueprint/blob/master/generators/entity-microfrontend/index.js

Thanks!

@mshima
Copy link
Member Author

mshima commented Oct 13, 2021

@avdev4j having the folder called entando-blueprint is not supported by yeoman, it should start with generator- otherwise namespace parsing becomes broken.
The namespace would resolve to entando-blueprint:entity-microfrontend instead of Users:anthonyviard:projects:entando:entando-blueprint:entity-microfrontend.

Anyway we should catch the error and log to debug.
Can you do the PR?

IMO the folder name should always match the package name https://github.com/entando/entando-blueprint/blob/0a15a091902a1a13dfdb4b8927ad2f3b445c977f/package.json#L2.
That's what happens to real packages.
If that is not doable, workarounds can be applied.

I guess this code should be applied only on a "real" extended JHipster generator.

Yes, but I don't see this as a problem, if the file is not found, it should ignore.

@avdev4j
Copy link
Contributor

avdev4j commented Oct 13, 2021

Great @mshima I can confirm you it's working by renaming the folder.

the strange thing is it's working like a charm on Linux OS with the old folder name. Anyway, it's was quite tricky but easy to fix.

I'll do PR in incoming days, I can reproduce it easily!

@pascalgrimaud
Copy link
Member

pascalgrimaud commented Oct 15, 2021

Is it generator- mandatory ?
Because it will impact existing blueprint. See https://github.com/jhipster/jhipster-kotlin
Or should it be renamed to generator-jhipster-kotlin ?

@mshima
Copy link
Member Author

mshima commented Oct 15, 2021

@pascalgrimaud

Is it generator- mandatory ?

Yes for the package name.

For the folder:
Short answer: yes.

Long answer: if needed.
The problem happens when relying on yeoman generated namespaces (when composing with a generator file using composeWith(require.resolve('../generators/client'))) and probably only happens at tests.

  • npm link will create a link named with package name. So running the generator our production is safe.
  • changing tests to don't rely on yeoman discovery namespaces will workaround the problem.
    Can be done by manually registering the generators, passing namespaces with the file path, or executing a custom discovery.

So if it's working as it is I don't see the need for changing, but for best practices new packages should start with generator-.

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