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

Some paths in webpack configs aren't matched on Windows #453

Closed
jgierer12 opened this issue Jan 5, 2018 · 5 comments
Closed

Some paths in webpack configs aren't matched on Windows #453

jgierer12 opened this issue Jan 5, 2018 · 5 comments
Labels

Comments

@jgierer12
Copy link
Contributor

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
I discovered this bug because routes are named *.chunk.*.js instead of route-*.js in the build output. This happens because Windows' backslash-delimited file paths (e.g. C:\Users\me\dev\project\src\routes\home\index.js aren't matched by these strings and regexes:

https://github.com/developit/preact-cli/blob/9eb46228922b05ecf88c0803a64f8fa5f04b2ab7/src/lib/webpack/webpack-client-config.js#L55-L58

There are probably more occurences of this throughout the webpack configs.

If the current behavior is a bug, please provide the steps to reproduce.

  1. preact create default test on Windows
  2. cd test, npm install
  3. npm run build
  4. In the build folder, route chunks aren't prefixed with route-

What is the expected behavior?
In this specific case, routes should be prefixed with route-, like on UNIX systems.
Generally, both / and \-delimited paths should be matched in webpack configs.

Please mention other relevant information.

  • node version 8.9.1
  • npm version 5.6.0
  • Operating system Windows 10 1709
@lukeed
Copy link
Member

lukeed commented Jan 5, 2018

Thanks for this! Can you try the following for me:

const path = require('path');
//...
let normal = path.posix.normalize(filename);
let relative = normal.replace(src, '');
return normal.includes('/routes/') && 'route-' + relative.replace(/(^\/(routes|components\/(routes|async))\/|(\/index)?\.js$)/g, '');

@lukeed lukeed added the bug label Jan 5, 2018
@jgierer12
Copy link
Contributor Author

That doesn't fix it, unfortunately. Relevant PR: nodejs/node#12700.

@lukeed
Copy link
Member

lukeed commented Jan 6, 2018

Ah, thanks! TIL :D Good reason I always avoided posix before.

How about this, the more cumbersome route:

const RGX = /[\\|\/]/g;
const toNorm = str => path.normalize(str).replace(RGX, '/');
// ...
filename = toNorm(filename);
if (!!~filename.indexOf('/routes/') return false;
let relative = filename.replace(toNorm(src), '');
return 'route-' + relative.replace(/(^\/(routes|components\/(routes|async))\/|(\/index)?\.js$)/g, '');

Sorry, no access to windows.

@jgierer12
Copy link
Contributor Author

jgierer12 commented Jan 6, 2018

It works if I replace !!~filename with !filename (and add the missing bracket).

I can work on this myself and submit a PR when I have enough time, if you'd prefer that.

Also, TIL about ~ 😃

@lukeed
Copy link
Member

lukeed commented Jan 6, 2018

Great! You're welcome to open a pull request for that, or else I'll do it later.

Sorry about that lol, on my phone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants