Skip to content

Conversation

@bolasblack
Copy link

Then we can write ../styles/app.css in caches.main

@NekR
Copy link
Owner

NekR commented May 3, 2017

Hi! We'll need to test this one or probably even write some tests for it. Can you verify it doesn't break any tests right now? Also, what's the rationally behind using such paths?

@bolasblack
Copy link
Author

Hi, I have executed npm test and there are no test failed.

In my case, I only use webpack to bundle javascript, other assets (for example css) was generated by gulp. The folder structure is like this:

.
├── assets
├── gulpfile.js
├── public
│   ├── index.html
│   ├── scripts
│   ├── styles
├── scripts
├── styles
└── webpack.config.js

My webpack.config.js write like this:

module.exports = {
  output: {
    path: sysPath.resolve('./public/scripts'),
    publicPath: '/scripts',
    filename: '[name].js',
  },
  plugins: [
    new OfflinePlugin({
      externals: [
        'index.html',
        'styles/app.css',
      ].concat(glob.sync(
        '**/*',
        { cwd: './assets/', nodir: true }
      )).map(a => '../' + a),
      ServiceWorker: {},
      AppCache: {},
    }),
  ],
}

With current code, the final public/scripts/appcache/manifest.appcache will include some path like this:

/scripts/../
/scripts/../styles/app.css

So I think concat path should use path.resolve instead of +

@bolasblack bolasblack changed the title use path.resolve to connect file path use path.join to connect file path May 5, 2017
@NekR
Copy link
Owner

NekR commented May 5, 2017

@bolasblack Okay, I see the reasons. But do really do changes and see if tests fail or not you need to build the project first. See CONTIBUTING.md

@GGAlanSmithee
Copy link
Collaborator

@NekR seems like all CI's are passing. Would you like to merge this? Othervise, maybe we can close it :)

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.

3 participants