search

FriendsOfSymfony / FOSCKEditorBundle

Provides a CKEditor integration for your Symfony project.
Other
518 stars 83 forks source link

Include CSS files when copying files with Encore #177

Closed bobvandevijver closed 5 years ago

bobvandevijver commented 5 years ago

Without the CSS files being copied into the build directory, a bunch of requests fail as they are missing. This change adds them to the build output.

Edit: I've also adjusted the rules to no longer include the samples directory.

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
kunicmarko20 commented 5 years ago

Thank you! cc @ktherage @Lyrkan could you review this?

kunicmarko20 commented 5 years ago

Ci failing is my fault, didn't merge from 2.x to master. Will do tonight

bobvandevijver commented 5 years ago

Currently, the documented rules also copy the samples folders into the build directory. We can prevent that from happening by using these rules as replacement for the first rule instead:

{from: './node_modules/ckeditor/', to: 'ckeditor/[path][name].[ext]', pattern: /\.(js|css)$/, includeSubdirectories: false},
{from: './node_modules/ckeditor/adapters', to: 'ckeditor/adapters/[path][name].[ext]'},
{from: './node_modules/ckeditor/plugins', to: 'ckeditor/plugins/[path][name].[ext]',

Shall I include this in this MR, or add a separate one?

kunicmarko20 commented 5 years ago

You can do it here.

bobvandevijver commented 5 years ago

This now generates a warning on ckeditor/plugins/emoji/emoji.json. 

 warning  in ./node_modules/ckeditor/plugins/emoji/emoji.json

Module parse failed: Unexpected token m in JSON at position 0 while parsing near 'module.exports = __w...'
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token m in JSON at position 0 while parsing near 'module.exports = __w...'

I'm not sure why the copyFiles function even tries to parse it, does anyone here have an idea to disable this?

Edit: Fixed with latest commit :)

Lyrkan commented 5 years ago

I'm not sure why the copyFiles function even tries to parse it, does anyone here have an idea to disable this?

Looks like a bug to me :(

We use something like require.context('!file-loader?<params>') to copy files, which means that all them are supposed to go through the file-loader only...

But Webpack 4 added a special treatment for .json file that collides with extra loaders (see https://github.com/webpack/webpack/issues/6586).

I'll try to see if something can be done in Encore to fix that but most solutions don't really apply to our case...

In the meantime I guess that your solution should work fine.

kunicmarko20 commented 5 years ago

Thank @bobvandevijver! Thanks @Lyrkan for reviewing again!

ktherage commented 5 years ago

@kunicmarko20 sorry I'm late :sweat_smile: I only seen your cc to me for review now

kunicmarko20 commented 5 years ago

np @ktherage, I guess change is fine?

ktherage commented 5 years ago

@kunicmarko20 Yes, in fact i didn't really mind myself about what may be imported and what may not. It just worked. I'm glad to see that this piece of doc leads more experimented people to make improvement on it.