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

Configurable webpack - rebase #31

Merged
merged 7 commits into from
Feb 4, 2021

Conversation

purplecabbage
Copy link
Member

@purplecabbage purplecabbage commented Feb 3, 2021

Description

Configurable webpack

Search action path for *webpack-config.js
search order is:

  1. /app-root/actions/actionname/
  2. /app-root/actions/
  3. /app-root/

Merge webpack config to include required props :

  • entry [actionPath, ...]
  • output { path, filename, ... }
  • resolve.extensions [.js, .json, ... ]
  • resolve.mainFields [ main, ... ]

Related Issue

Motivation and Context

extensible everything

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #31 (5ccf3da) into master (0356ac0) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #31   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          992      1038   +46     
  Branches       242       256   +14     
=========================================
+ Hits           992      1038   +46     
Impacted Files Coverage Δ
src/build-actions.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0356ac0...242ddd4. Read the comment docs.

Copy link
Member

@shazron shazron left a comment

Choose a reason for hiding this comment

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

There are warnings regarding "Assignment to function parameter 'filterActions'"
and also JSDoc warnings

Copy link
Member

@moritzraho moritzraho left a comment

Choose a reason for hiding this comment

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

apart from eslint warnings, code lgtm, will do some testing now

@moritzraho
Copy link
Member

moritzraho commented Feb 4, 2021

I see two things which are required to compile some of our sdks that should be further enforced:

  • libraryTarget: 'commonjs2', if config.output = {}, libraryTarget will get overwritten
  • minimize: false, if config.optimizations ={}, minimize: false will get overwritten

I think those two should be overwritten only if explicitly set by the user.

@moritzraho moritzraho mentioned this pull request Feb 4, 2021
10 tasks
@moritzraho
Copy link
Member

All good on my side now!
@purplecabbage feel free to give it a last review and merge

Here is a summary of my manual testing:

Setup:

  • new app with 5 actions (including asset-compute)

Flow:

  • test commands:
    • DEBUG=@adobe/aio-lib-runtime:action-builder* aio app build --skip-static
    • DEBUG=@adobe/aio-lib-runtime:action-builder* aio app run --skip-static
  • look at logs:
    • make sure the action builds
    • the config is the one expected
  • can invoke the action when applicable

Variations

  • no config file
  • using a full webpack-config.js in root only => make sure it's picked up
  • adding a different webpack-config.js in actions folder => make sure this is the one picked up
  • adding yet a different in specific actions
  • set entry files with relative paths to the config file
  • make sure there are no duplicates in config
  • custom package name (no __APP_PACKAGE__)
  • 2 packages

@purplecabbage purplecabbage merged commit 1ceca35 into adobe:master Feb 4, 2021
@purplecabbage purplecabbage deleted the config-webpack2 branch February 4, 2021 16:52
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