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

fix: Removed usage of loader-utils #1288

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

JonWallsten
Copy link
Contributor

@JonWallsten JonWallsten commented Apr 20, 2021

Remove usage of loader-utils:
#1286 (comment)

@JonWallsten
Copy link
Contributor Author

JonWallsten commented Apr 20, 2021

@alexander-akait: Is this how you meant?

Schema doesn't seem to be optional though:
image

Edit: Something i definitely wrong! 😅

emojis-list@^3.0.0:
version "3.0.0"
resolved "https://registry.npmjs.org/emojis-list/-/emojis-list-3.0.0.tgz#5570662046ad29e2e916e71aae260abdff4f6a78"
resolved "https://registry.yarnpkg.com/emojis-list/-/emojis-list-3.0.0.tgz#5570662046ad29e2e916e71aae260abdff4f6a78"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johnnyreilly Haven't used yarn or a lockfile for a long time. Are these changes okey?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

loader-utils@^1.0.2 is used by babel-loader I think. Probably has something to do with the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

babel-loader support old versions of webpack, they drop it in future, ideally we should improve getOptions on webpack side like: getOptions<T extends object>(schema: any): T;

Copy link
Member

Choose a reason for hiding this comment

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

It's definitely optional: https://github.com/webpack/webpack/blob/85fe6ac4f436384e2a20c7996c70f14e4172ccd8/lib/NormalModule.js#L495

It's our definition in this case that's causing the issue. But I'm fine with that. I'm hoping this will resolve it in future: webpack/webpack#13164

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexander-akait: If I'm not mistaken you need getOptions<T extends object>(schema?: any): T; to make it truly optional in strict mode. That's why I had to send in undefined. I can be wrong here though.

@JonWallsten
Copy link
Contributor Author

JonWallsten commented Apr 20, 2021

@alexander-akait:
image

I guess that has something to do with this?

image

src/index.ts Outdated
@@ -646,7 +643,7 @@ function makeSourceMap(
return {
output: outputText.replace(/^\/\/# sourceMappingURL=[^\r\n]*/gm, ''),
sourceMap: Object.assign(JSON.parse(sourceMapText), {
sources: [loaderUtils.getRemainingRequest(loaderContext)],
sources: loaderContext.remainingRequest,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not change it on string, keep in array, i.e. [loaderContext.remainingRequest]

Copy link
Contributor Author

@JonWallsten JonWallsten Apr 20, 2021

Choose a reason for hiding this comment

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

Of course! Now I feel stupid! 😅
Thanks!

@johnnyreilly johnnyreilly merged commit e571290 into TypeStrong:main Apr 20, 2021
Copy link
Contributor

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Great

@johnnyreilly
Copy link
Member

@jamestalton
Copy link

jamestalton commented Apr 21, 2021

Our storybook which is using ts-loader no longer shows any stories after this change.

The glob specified in main.js isn't correct.

{
    test: /\.(ts|tsx)$/,
    loader: 'ts-loader',
    options: {
        transpileOnly: true,
    },
},

@johnnyreilly
Copy link
Member

Hi,

Thanks for reporting. Can you confirm that 9.0.1 works without issue please?

@jamestalton
Copy link

Verified that ts-loader 9.0.1 works and 9.0.2 does not for our storybook.
We could always have something not setup right, but it has been working.
https://github.com/open-cluster-management/ui-components

@alexander-akait
Copy link
Contributor

alexander-akait commented Apr 22, 2021

@jamestalton most likely you have two webpack versions, run npm ls webpack/yarn why webpack

@jamestalton
Copy link

Looks like just webpack@4.46.0

npm ls webpack

@open-cluster-management/ui-components@0.65.0 /Users/jtalton/projects/ui-components
├─┬ @storybook/addon-essentials@6.2.8
│ └─┬ @storybook/addon-docs@6.2.8
│   └─┬ @storybook/builder-webpack4@6.2.8
│     └── webpack@4.46.0 deduped
├─┬ @storybook/react@6.2.8
│ ├─┬ @storybook/core-common@6.2.8
│ │ └── webpack@4.46.0 deduped
│ ├─┬ @storybook/core@6.2.8
│ │ └─┬ @storybook/core-server@6.2.8
│ │   └── webpack@4.46.0 deduped
│ └── webpack@4.46.0
└─┬ ts-loader@9.0.2
  └── webpack@4.46.0 deduped

@JonWallsten
Copy link
Contributor Author

@jamestalton: ts-loader 9.x.x is for Webpack >= 5. That's probably why you get the issue!

@gligoran
Copy link

I'm getting the following error with ts-loader 9.0.2 or later:

👀  ./src/index.ts
Module build failed (from ./node_modules/ts-loader/index.js):
TypeError: loaderContext.getOptions is not a function
    at getLoaderOptions (/Users/gligoran/dev/my-project/node_modules/ts-loader/dist/index.js:90:41)
    at Object.loader (/Users/gligoran/dev/my-project/node_modules/ts-loader/dist/index.js:14:21)
Error: webpack returned an error. Try configuring `entry` in your webpack config relative to the current working directory, or setting `context = __dirname` in your webpack config.

The suggestion at the end does nothing.

The same app does work with ts-loader 9.0.1.

@sandro768
Copy link

I'm getting the following error with ts-loader 9.0.2 or later:

👀  ./src/index.ts
Module build failed (from ./node_modules/ts-loader/index.js):
TypeError: loaderContext.getOptions is not a function
    at getLoaderOptions (/Users/gligoran/dev/my-project/node_modules/ts-loader/dist/index.js:90:41)
    at Object.loader (/Users/gligoran/dev/my-project/node_modules/ts-loader/dist/index.js:14:21)
Error: webpack returned an error. Try configuring `entry` in your webpack config relative to the current working directory, or setting `context = __dirname` in your webpack config.

The suggestion at the end does nothing.

The same app does work with ts-loader 9.0.1.

I've got the same issue

@JonWallsten
Copy link
Contributor Author

@gligoran, @sandro768: You most probably have webpack@4.x.x in you project.

@gligoran
Copy link

@gligoran, @sandro768: You most probably have webpack@4.x.x in you project.

Oh, sorry. I've seen that comment about this a bit up and wanted to mention it in mine, but forgot.

I was using webpack 4 and upgraded to 5. Here are my relevant devDeps:

+-- ts-loader@9.0.1
+-- typescript@4.2.4
+-- webpack@5.36.0
`-- webpack-cli@4.6.0

If I upgrade to tsloader@9.0.2 or above it produces the error in my previous comment.

Here's my webpack.config.js:

const path = require('path');

const mode = process.env.NODE_ENV || 'production';

module.exports = {
  mode,
  entry: './src/index.ts',
  target: 'webworker',
  module: {
    rules: [
      {
        test: /\.tsx?$/,
        loader: 'ts-loader',
        options: {
          transpileOnly: true,
        },
      },
    ],
  },
  resolve: {
    extensions: ['.ts', '.js'],
  },
  output: {
    filename: `worker.js`,
    path: path.join(__dirname, 'dist'),
  },
};

Could this be a problem with upgrading from 4 in terms of old config style?

@alexander-akait
Copy link
Contributor

@gligoran run yarn why webpack/npm ls webpack

@gligoran
Copy link

@gligoran run yarn why webpack/npm ls webpack

Here's what that returns (just updated to the newest version of webpack):

 $ npm ls --depth=10 webpack
`-- webpack@5.36.1

I also ran this, if there's any other webpack stuff that might lend a better insight:

$ npm ls --depth=10 | grep webpack
+-- webpack@5.36.1
| +-- terser-webpack-plugin@5.1.1
| `-- webpack-sources@2.2.0
`-- webpack-cli@4.6.0
  +-- @webpack-cli/configtest@1.0.2
  +-- @webpack-cli/info@1.2.3
  +-- @webpack-cli/serve@1.3.1
  `-- webpack-merge@5.7.3

@alexander-akait
Copy link
Contributor

@gligoran What is the error?

@gligoran
Copy link

gligoran commented May 4, 2021

@gligoran What is the error?

I've posted it here: #1288 (comment)

Or is this not detailed enough? If so, how can I provide more info?

@alexander-akait
Copy link
Contributor

alexander-akait commented May 4, 2021

@gligoran do you use thread-loader, this error means you still on webpack v4 or something change loader context (thread-loader), anyway if you provide repo I can look

@gligoran
Copy link

gligoran commented May 7, 2021

@gligoran do you use thread-loader, this error means you still on webpack v4 or something change loader context (thread-loader), anyway if you provide repo I can look

It seems that wrangler replaces the installed webpack version. So, it seems we're still running webpack 4 regardless of what's installed in the repo itself.

@cinderblock
Copy link

cinderblock commented Aug 30, 2021

I am getting this error with Webpack 5 and ts-loader@9.2.5 (and ts-loader@9.0.2). Downgrading to 9.0.1 gets me past this error.

ERROR in ./main.tsx
Module build failed (from ../node_modules/ts-loader/index.js):
TypeError: loaderContext.getOptions is not a function
    at getLoaderOptions (C:\Users\camer\git\my-project\node_modules\ts-loader\dist\index.js:90:41)
    at Object.loader (C:\Users\camer\git\my-project\node_modules\ts-loader\dist\index.js:14:21)

Update

Wow. So, while I did have Webpack 5 installed, something else had installed webpack 4 and a higher level folder. webpack-cli was grabbing version 4 and running with that, causing the problems above.

undergroundwires added a commit to undergroundwires/privacy.sexy that referenced this pull request Dec 31, 2021
Upgrade to v5.x using `vue upgrade --next`.

Update `vue.config.js` to import and use `defineConfig`, because it
provides type safety and created by Vue CLI 5 as default.

Vue CLI 5.x upgrades from webpack 4 to 5. It causes some issues that this
commit attemps to fix:

1. Fail due to webpack resolving of Ace.
   Third-party dependency (code editor) Ace uses legacy `file-loader`
   for webpack resolving. It's not supported in webpack 5. So change it
   with manual imports.
   Refs: ajaxorg/ace-builds#211, ajaxorg/ace-builds#221.

2. Wehpack drops polyfilling node core modules (`path`, `fs`, etc.).
   Webpack does not polyfill those modules by default anymore. This is
   good because they did not need browser polyfilling as they are
   used in desktop version only and resolved already by Electron.
   To resolve errors (using webpack recommendations):
    - Add typeof check around `process` variable.
    - Tell webpack explicitly to ignore used node modules.

3. Fail due to legacy dependency of vue-cli-plugin-electron-builder.
   This plugin is used for electron builds and development. It still
   uses webpack 4 that leads to failed builds.
   Downgrading `ts-loader` to latest version which has support for
   `loader-utils` solves the problem (TypeStrong/ts-loader#1288).
   Related issue: nklayman/vue-cli-plugin-electron-builder#1625

4. Compilation fails due to webpack loading of `fsevents` on macOS.
   This happens only when running `vue-cli-service test:unit` command
   (used in integration tests and unit tests). Other builds work fine.
   Refs: yan-foto/electron-reload#71,
     nklayman/vue-cli-plugin-electron-builder#712,
     nklayman/vue-cli-plugin-electron-builder#1333
LarrMarburger pushed a commit to LarrMarburger/privacy.sexy that referenced this pull request Nov 16, 2023
Upgrade to v5.x using `vue upgrade --next`.

Update `vue.config.js` to import and use `defineConfig`, because it
provides type safety and created by Vue CLI 5 as default.

Vue CLI 5.x upgrades from webpack 4 to 5. It causes some issues that this
commit attemps to fix:

1. Fail due to webpack resolving of Ace.
   Third-party dependency (code editor) Ace uses legacy `file-loader`
   for webpack resolving. It's not supported in webpack 5. So change it
   with manual imports.
   Refs: ajaxorg/ace-builds#211, ajaxorg/ace-builds#221.

2. Wehpack drops polyfilling node core modules (`path`, `fs`, etc.).
   Webpack does not polyfill those modules by default anymore. This is
   good because they did not need browser polyfilling as they are
   used in desktop version only and resolved already by Electron.
   To resolve errors (using webpack recommendations):
    - Add typeof check around `process` variable.
    - Tell webpack explicitly to ignore used node modules.

3. Fail due to legacy dependency of vue-cli-plugin-electron-builder.
   This plugin is used for electron builds and development. It still
   uses webpack 4 that leads to failed builds.
   Downgrading `ts-loader` to latest version which has support for
   `loader-utils` solves the problem (TypeStrong/ts-loader#1288).
   Related issue: nklayman/vue-cli-plugin-electron-builder#1625

4. Compilation fails due to webpack loading of `fsevents` on macOS.
   This happens only when running `vue-cli-service test:unit` command
   (used in integration tests and unit tests). Other builds work fine.
   Refs: yan-foto/electron-reload#71,
     nklayman/vue-cli-plugin-electron-builder#712,
     nklayman/vue-cli-plugin-electron-builder#1333
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.

7 participants