-
-
Notifications
You must be signed in to change notification settings - Fork 198
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 support for pnpm and Yarn PnP #1142
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
name: Testing apps | ||
on: [push, pull_request] | ||
|
||
jobs: | ||
testing_app: | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
test_app: | ||
- name: npm | ||
working_directory: test_apps/npm | ||
script: | | ||
npm install --ci | ||
npm add --save-dev ../../webpack-encore.tgz | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Installing the |
||
npm run encore dev | ||
npm run encore production | ||
|
||
- name: pnpm | ||
working_directory: test_apps/pnpm | ||
script: | | ||
pnpm install --frozen-lockfile | ||
pnpm add --save-dev ../../webpack-encore.tgz | ||
pnpm run encore dev | ||
pnpm run encore production | ||
|
||
- name: Yarn Plug'n'Play | ||
working_directory: test_apps/yarn_pnp | ||
script: | | ||
yarn set version berry | ||
yarn install --frozen-lockfile | ||
yarn add --dev ../../webpack-encore.tgz | ||
yarn run encore dev | ||
yarn run encore production | ||
|
||
name: ${{ matrix.test_app.name }} | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v2.0.0 | ||
|
||
- name: Node ${{matrix.node-versions}} | ||
uses: actions/setup-node@v2 | ||
with: | ||
node-version: '14' | ||
|
||
- if: matrix.test_app.name == 'pnpm' | ||
name: Install pnpm | ||
uses: pnpm/action-setup@v2 | ||
with: | ||
version: latest | ||
|
||
- name: Packing Encore | ||
run: yarn pack --filename webpack-encore.tgz | ||
|
||
- name: Running script | ||
working-directory: ${{ matrix.test_app.working_directory }} | ||
run: ${{ matrix.test_app.script }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,10 +45,9 @@ | |
"semver": "^7.3.2", | ||
"style-loader": "^3.3.0", | ||
"sync-rpc": "^1.3.6", | ||
"tapable": "^2.2.1", | ||
"terser-webpack-plugin": "^5.3.0", | ||
"tmp": "^0.2.1", | ||
"webpack": "^5.72", | ||
"webpack-cli": "^4.9.1", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this mean that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For If you take a look to
Maybe Yarn PNP and pnpm have a different way to deal with dependencies's peer dependencies. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. But this is necessary anyway as soon as you want to install an extra webpack plugin and being sure that it uses the same webpack version than the one used for the build, which is a must-have. If we want to hide the tool, we need to hide it entirely, which is not the purpose of Encore as it is a configurable tool. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The situation about A) (?) For non-PNP, you will or won't need to install Will the code in Webpack's executable (https://github.com/webpack/webpack/blob/9fcaa243573005d6fdece9a3f8d89a0e8b399613/bin/webpack.js#L90-L118) effectively enforce whether or not the user will need to install it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A) For npm (8), If
And if I type
B) For Yarn PNP, if
And when I install
C) Hum, same A) now, I don't know what changed. 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For A) and C), adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. webpack defines the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay I think I found the issue, when running I will push ASAP. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in cc @weaverryan |
||
"webpack-dev-server": "^4.8.0", | ||
"yargs-parser": "^21.0.0" | ||
}, | ||
|
@@ -98,9 +97,135 @@ | |
"vue": "^3.2.14", | ||
"vue-loader": "^17.0.0", | ||
"vue-template-compiler": "^2.5", | ||
"webpack": "^5.72", | ||
"webpack-cli": "^4.9.1", | ||
"webpack-notifier": "^1.15.0", | ||
"zombie": "^6.1.4" | ||
}, | ||
"peerDependencies": { | ||
"@babel/plugin-proposal-class-properties": "^7.0.0", | ||
"@babel/plugin-transform-react-jsx": "^7.12.11", | ||
"@babel/preset-react": "^7.0.0", | ||
"@babel/preset-typescript": "^7.0.0", | ||
"@symfony/stimulus-bridge": "^3.0.0", | ||
"@vue/babel-helper-vue-jsx-merge-props": "^1.0.0", | ||
"@vue/babel-preset-jsx": "^1.0.0", | ||
"@vue/compiler-sfc": "^3.0.2", | ||
"eslint": "^8.0.0", | ||
"eslint-webpack-plugin": "^3.1.0", | ||
"file-loader": "^6.0.0", | ||
"fork-ts-checker-webpack-plugin": "^7.0.0", | ||
"handlebars": "^4.7.7", | ||
"handlebars-loader": "^1.7.0", | ||
"less": "^4.0.0", | ||
"less-loader": "^11.0.0", | ||
"postcss": "^8.3.0", | ||
"postcss-loader": "^7.0.0", | ||
"sass": "^1.17.0", | ||
"sass-loader": "^13.0.0", | ||
"stylus": "^0.58.1", | ||
"stylus-loader": "^7.0.0", | ||
"ts-loader": "^9.0.0", | ||
"typescript": "^4.2.2", | ||
"vue": "^3.2.14", | ||
"vue-loader": "^17.0.0", | ||
"vue-template-compiler": "^2.5", | ||
"webpack": "^5.72", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dev dependencies of webpack have no impact when developing Encore. We are not developing webpack in such situation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do I have anything to change here? ATM |
||
"webpack-cli": "^4.9.1", | ||
"webpack-notifier": "^1.15.0" | ||
}, | ||
"peerDependenciesMeta": { | ||
"@babel/plugin-proposal-class-properties": { | ||
"optional": true | ||
}, | ||
"@babel/plugin-transform-react-jsx": { | ||
"optional": true | ||
}, | ||
"@babel/preset-react": { | ||
"optional": true | ||
}, | ||
"@babel/preset-typescript": { | ||
"optional": true | ||
}, | ||
"@symfony/stimulus-bridge": { | ||
"optional": true | ||
}, | ||
"@vue/babel-helper-vue-jsx-merge-props": { | ||
"optional": true | ||
}, | ||
"@vue/babel-preset-jsx": { | ||
"optional": true | ||
}, | ||
"@vue/compiler-sfc": { | ||
"optional": true | ||
}, | ||
"eslint": { | ||
"optional": true | ||
}, | ||
"eslint-webpack-plugin": { | ||
"optional": true | ||
}, | ||
"file-loader": { | ||
"optional": true | ||
}, | ||
"fork-ts-checker-webpack-plugin": { | ||
"optional": true | ||
}, | ||
"handlebars": { | ||
"optional": true | ||
}, | ||
"handlebars-loader": { | ||
"optional": true | ||
}, | ||
"less": { | ||
"optional": true | ||
}, | ||
"less-loader": { | ||
"optional": true | ||
}, | ||
"postcss": { | ||
"optional": true | ||
}, | ||
"postcss-loader": { | ||
"optional": true | ||
}, | ||
"sass": { | ||
"optional": true | ||
}, | ||
"sass-loader": { | ||
"optional": true | ||
}, | ||
"stylus": { | ||
"optional": true | ||
}, | ||
"stylus-loader": { | ||
"optional": true | ||
}, | ||
"ts-loader": { | ||
"optional": true | ||
}, | ||
"typescript": { | ||
"optional": true | ||
}, | ||
"vue": { | ||
"optional": true | ||
}, | ||
"vue-loader": { | ||
"optional": true | ||
}, | ||
"vue-template-compiler": { | ||
"optional": true | ||
}, | ||
"webpack": { | ||
"optional": false | ||
}, | ||
"webpack-cli": { | ||
"optional": false | ||
}, | ||
"webpack-notifier": { | ||
"optional": true | ||
} | ||
}, | ||
"files": [ | ||
"lib/", | ||
"bin/", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more descriptive name we could use here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, this workflow means to test the installation and run of Encore on real projects/app.
Do you have something in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may have just suggested a good one
name: Testing installation in real apps