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

breaks when used in webpack <=3, with svelte v3 #89

Closed
0gust1 opened this issue May 10, 2019 · 8 comments
Closed

breaks when used in webpack <=3, with svelte v3 #89

0gust1 opened this issue May 10, 2019 · 8 comments

Comments

@0gust1
Copy link

0gust1 commented May 10, 2019

Sorry for the annoying issue :-/

TLDR :

It's a cross bug between :

  • the changes between svelte v2 and svelte v3 compiler strategy in options handling
  • a mocha test written the old way.
  • the usage of both this.options and loader-utils.getOptions(this) to handle options.
    ** this.options was deprecated in webpack3 and removed in webpack4.
    ** loader-utils.getOptions(this) is the right way to do. It appeared in version 1.0.0 (21 Feb 2017) of loader-utils and seems supported since webpack v1

edit: proposed pull request : #90
edit: related issue : #88
edit: possibly related issue/pr : #87

Below, all the analysis I did investigating the issue. Sorry, it's a bit verbose.


Context :
Our big project is stuck to webpack 3 at the moment (due to a bug in webpack 4 that will be resolved in webpack 5).

This loader seems to break in webpack3. We have been able to reconstruct a minimal case (using the svelte-app template and making a dichotomy with webpack3 vs webpack4)

  • with webpack3, the loader breaks at the same line as our big project :
    let { js, css, warnings } = normalize(compile(processed.toString(), compileOptions));
  • with webpack4 everything compile fine.

The kind of errors we have :

ERROR in ./src/App.svelte
Module build failed: Error: Error: Error: Unrecognized option 'entry'
    at preprocess.then.catch.err (/ABSOLUTE_PATH/test_svelte3_webpack3/node_modules/svelte-loader/index.js:182:12)
 @ ./src/main.js 1:0-31

On our other project, it's not entry but context.


After further analysis, it seems that the svelte V3 compiler doesn't like the configuration feeded to it when it comes from webpack 3. The validate_options() function of the compiler reject it.

Config is inited here :

const options = Object.assign({}, this.options, getOptions(this));
where this.options is :

  • with webpack 3 : the webpack big config object
  • with webpack 4 : undefined (because it was finally deprecated)

Config is transformed along the way (into a compileOptions object) and, after that, it's used here :

let { js, css, warnings } = normalize(compile(processed.toString(), compileOptions));

In the minimal test case we have :

compileOptions with webpack 3 and svelte 3

{
  "filename": "/workspace_absolute_path/test_svelte3_webpack3/src/App.svelte",
  "format": "esm",
  "entry": "./src/main.js",
  "module": {
    "rules": [
      { "test": {}, "use": [{ "loader": "svelte-loader", "options": {} }] }
    ],
    "unknownContextRequest": ".",
    "unknownContextRegExp": false,
    "unknownContextRecursive": true,
    "unknownContextCritical": true,
    "exprContextRequest": ".",
    "exprContextRegExp": false,
    "exprContextRecursive": true,
    "exprContextCritical": true,
    "wrappedContextRegExp": {},
    "wrappedContextRecursive": true,
    "wrappedContextCritical": false,
    "strictExportPresence": false,
    "strictThisContextOnImports": false,
    "unsafeCache": true
  },
  "output": {
    "path": "/workspace_absolute_path/test_svelte3_webpack3/public",
    "filename": "bundle.js",
    "chunkFilename": "[id].bundle.js",
    "library": "",
    "hotUpdateFunction": "webpackHotUpdate",
    "jsonpFunction": "webpackJsonp",
    "libraryTarget": "var",
    "sourceMapFilename": "[file].map[query]",
    "hotUpdateChunkFilename": "[id].[hash].hot-update.js",
    "hotUpdateMainFilename": "[hash].hot-update.json",
    "crossOriginLoading": false,
    "jsonpScriptType": "text/javascript",
    "chunkLoadTimeout": 120000,
    "hashFunction": "md5",
    "hashDigest": "hex",
    "hashDigestLength": 20,
    "devtoolLineToLine": false,
    "strictModuleExceptionHandling": false
  },
  "context": "/workspace_absolute_path/test_svelte3_webpack3",
  "devtool": false,
  "cache": true,
  "target": "web",
  "node": {
    "console": false,
    "process": true,
    "global": true,
    "Buffer": true,
    "setImmediate": true,
    "__filename": "mock",
    "__dirname": "mock"
  },
  "performance": {
    "maxAssetSize": 250000,
    "maxEntrypointSize": 250000,
    "hints": false
  },
  "resolve": {
    "unsafeCache": true,
    "modules": ["node_modules"],
    "extensions": [".js", ".json"],
    "mainFiles": ["index"],
    "aliasFields": ["browser"],
    "mainFields": ["browser", "module", "main"],
    "cacheWithContext": false
  },
  "resolveLoader": {
    "unsafeCache": true,
    "mainFields": ["loader", "main"],
    "extensions": [".js", ".json"],
    "mainFiles": ["index"],
    "cacheWithContext": false
  }
}

compileOptions with webpack 4 and svelte 3

{"filename":"/workspace_absolute_path/test_svelte3_webpack3/src/App.svelte","format":"esm"}
@0gust1 0gust1 changed the title This loader seems to break when used in webpack3 This loader seems to break when used in webpack3, with svelte v3 May 10, 2019
@0gust1
Copy link
Author

0gust1 commented May 10, 2019

Corresponding PR : #90

@0gust1
Copy link
Author

0gust1 commented May 11, 2019

Summed up explanation :

  • in a webpack <=3.x + svelte v3 config, the loader pass a misconfigured option object to the svelte compiler and makes it choke (because the svelte v3 compiler check the content of the option object before using it)
  • You don't have this problem with webpack >= 4.x + svelte v3, because webpack removed the use of this.options (which now returns undefined), so the final option object is filled with loader-utils.getOptions(this) content only.
  • You don't have this problem with webpack <= 3.x + svelte v2, because the svelte v2 compiler is less strict about the content of the option object it digests.

I can push the code of the minimal test-case project I used, if needed.

@0gust1 0gust1 changed the title This loader seems to break when used in webpack3, with svelte v3 This loader seems to break when used in webpack <=3, with svelte v3 May 11, 2019
@mrkishi
Copy link
Member

mrkishi commented May 11, 2019

Is this.options needed? getOptions() uses this.query and it seems to be supported since webpack v1.

@esarbanis do you remember why you added this.options here?

@0gust1
Copy link
Author

0gust1 commented May 11, 2019

@mrkishi thanks for the precision ! I confess I lacked some energy to dig on the evolution of the API on webpack side. I will do some tests on my minimal test-case. I just hope we won't hit some edge case, trying to get rid of this.options, webpackism is hairy sometimes ^^

@0gust1
Copy link
Author

0gust1 commented May 13, 2019

Some links to webpack loader apis :

I still don't know why this.options was used, but it worth noting that getOptions has his own sets of bugs in the past (apparently linked to "query paremeters" expressed options).

getOptions appeared in version 1.0.0 (21 Feb 2017) of loader-utils.

I'll make some tests and push changes on my PR today (which should be muuuuch cleaner :) ). Thanks !

@0gust1
Copy link
Author

0gust1 commented May 13, 2019

@mrkishi , @esarbanis : I did some analysis again.

tldr: removing this.options does the job, but breaks a test that maybe should rewritten.

  • It breaks the should preprocess successfully and should not preprocess successfully
  • but I tested in my minimal setup (implementing the same preprocess as in the test), and the preprocess works (I can see {width:50px;height:50px} in the preprocessed result and final compiled result).

New test in PR soon.

@0gust1
Copy link
Author

0gust1 commented May 13, 2019

@mrkishi @esarbanis @Rich-Harris @ekhaled @Conduitry

CI is green, everything seems OK. Can you check #90 ?

@0gust1 0gust1 changed the title This loader seems to break when used in webpack <=3, with svelte v3 This loader breaks when used in webpack <=3, with svelte v3 May 13, 2019
@0gust1 0gust1 changed the title This loader breaks when used in webpack <=3, with svelte v3 Bug when used in webpack <=3, with svelte v3 May 13, 2019
@0gust1 0gust1 changed the title Bug when used in webpack <=3, with svelte v3 breaks when used in webpack <=3, with svelte v3 May 15, 2019
@Rich-Harris
Copy link
Member

Released 2.13.4 with the fix — thanks

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

No branches or pull requests

3 participants