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

Enable running npm project without maven #5985

Merged
merged 5 commits into from
Jun 28, 2019
Merged

Conversation

caalador
Copy link
Contributor

@caalador caalador commented Jun 27, 2019

Enable generation and copying of all
required files without the need to execute maven.

Note! to run in npm mode without maven
the file src/main/resources/META-INF/VAADIN/config/flow-build-info.json
with the content { "compatibilityMode": false } needs to exist in the project.

Fixes #5927


This change is Reviewable

Enable generation and copying of all
required files without the need to execute maven.

Note! to run in npm mode without maven
the file `src/main/resources/META-INF/VAADIN/config/flow-build-info.json`
with the content `{ "compatibilityMode": false }`  needs to exist in the project.
@project-bot project-bot bot added this to Review in progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Jun 27, 2019
Copy link
Contributor

@denis-anisimov denis-anisimov left a comment

Choose a reason for hiding this comment

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

Reviewed 11 of 12 files at r1, 1 of 1 files at r3.
Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)

a discussion (no related file):

Note! to run in npm mode without maven
the file src/main/resources/META-INF/VAADIN/config/flow-build-info.json
with the content { "compatibilityMode": false } needs to exist in the project.

May I just set a property to run it in npm ?



flow-server/src/main/java/com/vaadin/flow/server/startup/DevModeInitializer.java, line 217 at r3 (raw file):

copyFrontendFilesFromJar

from Jars (plural) ?

Only jars ?
No need to care about compiled classes ? (e.g. classes folder)

Copy link
Contributor Author

@caalador caalador left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…

Note! to run in npm mode without maven
the file src/main/resources/META-INF/VAADIN/config/flow-build-info.json
with the content { "compatibilityMode": false } needs to exist in the project.

May I just set a property to run it in npm ?

You may just set it as a property, but in this case it becomes a question that where would you put it?
This is one solution that can be pushed to the version-control system and will therfore always work.
Having it in maven won't work as we don't in this case run maven.

You could create a servlet with the property as a servlet parameter, but as we generally don't need those...
Also it should work having it in application.property for spring projects, but the file recommendation is so that we could have all starters work the same way.



flow-server/src/main/java/com/vaadin/flow/server/startup/DevModeInitializer.java, line 217 at r3 (raw file):

Previously, denis-anisimov (Denis) wrote…
copyFrontendFilesFromJar

from Jars (plural) ?

Only jars ?
No need to care about compiled classes ? (e.g. classes folder)

Done.

Copy link
Contributor Author

@caalador caalador left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)


flow-server/src/main/java/com/vaadin/flow/server/startup/DevModeInitializer.java, line 217 at r3 (raw file):

Previously, caalador wrote…

Done.

Also yes we only copy .js and .css files from the jar files so that they are available for webpack.

@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 1 issue

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR JarContentsManager.java#L124: Return an empty array instead of null. rule

Copy link
Contributor

@denis-anisimov denis-anisimov left a comment

Choose a reason for hiding this comment

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

:lgtm:

I'm fine with this.

Reviewed 1 of 1 files at r4.
Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained

a discussion (no related file):

Previously, caalador wrote…

You may just set it as a property, but in this case it becomes a question that where would you put it?
This is one solution that can be pushed to the version-control system and will therfore always work.
Having it in maven won't work as we don't in this case run maven.

You could create a servlet with the property as a servlet parameter, but as we generally don't need those...
Also it should work having it in application.property for spring projects, but the file recommendation is so that we could have all starters work the same way.

I'm just asking whether I may avoid flow-build-info.json modification but just set a property when I run a project.
Without any other context.

Normally I'm not aware about flow-build-info.json at all (sure, it's easier for us if we want to store it in VCS).
But I know that there is a property which I may use to enable npm (that's the only thing I need : to be able to run project in npm mode instead of bower since by default it's bower mode and I have to do something to start it in npm).

So if I may use a property without knowing anything about flow-build-info.json that's OK for me.


OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Review in progress to Reviewer approved Jun 27, 2019
@mehdi-vaadin mehdi-vaadin merged commit a87f49a into master Jun 28, 2019
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Reviewer approved to Done - pending release Jun 28, 2019
@mehdi-vaadin mehdi-vaadin deleted the issues/5927_clean_run branch June 28, 2019 06:40
@ujoni ujoni added this to the 2.0.2 milestone Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging this pull request may close these issues.

"Run on server" or running Sprint Boot Application class runs compatibility mode in 14
5 participants