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

10838 cbyrd upgrade primevue #10990

Merged
merged 4 commits into from
May 31, 2024
Merged

10838 cbyrd upgrade primevue #10990

merged 4 commits into from
May 31, 2024

Conversation

chrabyrd
Copy link
Contributor

Types of changes

  • Bugfix (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 not work as expected)

Description of Change

Issues Solved

#10838

@chrabyrd
Copy link
Contributor Author

beta.4 causes some heinous webpack issues, so backing off to beta.3 for now

Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

If you don't mind, let's go ahead and solve the webpack issue with the new beta while we're here. I'm showing it's solved with:

diff --git a/webpack/webpack.common.js b/webpack/webpack.common.js
index 68c779fd84..b5d0d1895b 100644
--- a/webpack/webpack.common.js
+++ b/webpack/webpack.common.js
@@ -305,7 +305,7 @@ module.exports = () => {
                 module: {
                     rules: [
                         {
-                            test: /\.tsx?$/,
+                            test: /\.m?tsx?$/,
                             exclude: /node_modules/,
                             loader: Path.join(PROJECT_RELATIVE_NODE_MODULES_PATH, 'ts-loader'),
                             options: {
@@ -319,12 +319,7 @@ module.exports = () => {
                             loader: Path.join(PROJECT_RELATIVE_NODE_MODULES_PATH, 'vue-loader'),
                         },
                         {
-                            test: /\.mjs$/,
-                            include: /node_modules/,
-                            type: 'javascript/auto',
-                        },
-                        {
-                            test: /\.js$/,
+                            test: /\.m?js$/,
                             exclude: [/node_modules/, /load-component-dependencies/],
                             loader: Path.join(PROJECT_RELATIVE_NODE_MODULES_PATH, 'babel-loader'),
                             options: {

(once copied to project template, of course)

@jacobtylerwalls
Copy link
Member

Taking another look, I may need to retest something before I declare that a success ...

@jacobtylerwalls
Copy link
Member

Ah, I think I just ran my test case incorrectly. Seems like what changed in the new beta is the .js -> .mjs, which seems like something we might need to handle in our config, but staying on beta3 is fine for now. Approving!

@chrabyrd
Copy link
Contributor Author

Ah yeah there's a webpack workaround but it hits what I believe is an error in the beta.4 release

We'll see what they say, but yeah to unblock for now I think merging beta.3 is the way to go 👍 . Will file a ticket for the appropriate webpack tweak.

@chrabyrd chrabyrd merged commit 65a5b0c into dev/7.6.x May 31, 2024
7 checks passed
@chrabyrd chrabyrd deleted the 10838-cbyrd-upgrade-primevue branch May 31, 2024 17:55
jacobtylerwalls added a commit that referenced this pull request Jun 17, 2024
…ade-primevue"

This reverts commit 65a5b0c, reversing
changes made to 326af3f.
jacobtylerwalls added a commit that referenced this pull request Jun 17, 2024
…ade-primevue"

This reverts commit 65a5b0c, reversing
changes made to 326af3f.
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.

2 participants