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

feat: opt-out web components from package.json #20392

Merged
merged 14 commits into from
Nov 7, 2024

Conversation

tltv
Copy link
Member

@tltv tltv commented Nov 1, 2024

Adds new property npm.excludeWebComponents (or npmExcludeWebComponents in Maven configurations). By default, it's false and everything works as before. true will exclude all web component dependencies from package.json for development mode (Vite/dev bundle) and production bundle build. Excluded dependencies are all Vaadin core components (e.g. button, grid, login, etc.) and commercial components (e.g. charts, rich-text-editor, etc.), but not lumo/material themes.

RelatedTo: #19948

Adds new property `vaadin.include.web.component.npm.packages` (or `includeWebComponentNpmPackages` in Maven configurations). By default, it's `true` and everything works as before. `false` will exclude all web component dependencies from `package.json` for development mode (Vite/dev bundle) and production bundle build. Excluded dependencies are all Vaadin core components (e.g. button, grid, login, etc.) and commercial components (e.g. charts, rich-text-editor, etc.), but not lumo/material themes.

RelatedTo: #19948
Copy link

github-actions bot commented Nov 1, 2024

Test Results

1 142 files  ± 0  1 142 suites  ±0   1h 26m 45s ⏱️ +31s
7 483 tests + 6  7 433 ✅ + 6  50 💤 ±0  0 ❌ ±0 
7 850 runs  +41  7 790 ✅ +40  60 💤 +1  0 ❌ ±0 

Results for commit d8364f3. ± Comparison against base commit 9378d46.

♻️ This comment has been updated with latest results.

@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Nov 4, 2024
@tltv tltv marked this pull request as ready for review November 4, 2024 08:36
Fills exclusions always when includeWebComponents=false. Makes it behave same by all callers. Updated couple tests.
@caalador
Copy link
Contributor

caalador commented Nov 7, 2024

Just running a project with mvn -Dnpm.excludeWebComponents=true or having

<configuration>
  <npmExcludeWebComponents>true</npmExcludeWebComponents>
</configuration>

I still get in my package.json the lines:

    "@vaadin/react-components": "24.6.0-alpha7",
    "@vaadin/react-components-pro": "24.6.0-alpha7",

Am I missing something or is the value not propagated correctly?

@tltv
Copy link
Member Author

tltv commented Nov 7, 2024

Am I missing something or is the value not propagated correctly?

Try again. Now it should be written in build-info from maven configs and delegated forward.
It has worked for me with application.properties and just now also with maven prop. Need try with -Dnpm.excludeWebComponents=true too

Update: works for me with latest change. Did you exclude all core component dependencies plus copilot? Exclude copilot like this (this is just temporary and copilot needs to be updated after this PR is ready):

        <dependency>
            <groupId>com.vaadin</groupId>
            <artifactId>vaadin-dev</artifactId>
            <exclusions>
                <exclusion>
                    <groupId>com.vaadin</groupId>
                    <artifactId>copilot</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

@vaadin-bot vaadin-bot added +0.1.0 and removed +0.0.1 labels Nov 7, 2024
Copy link

sonarcloud bot commented Nov 7, 2024

@tltv tltv merged commit fd16ca6 into main Nov 7, 2024
26 checks passed
@tltv tltv deleted the feat/19948-optout-web-components branch November 7, 2024 13:01
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.6.0.alpha3 and is also targeting the upcoming stable 24.6.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants