search

fusepilot / create-cep-extension

A near zero config approach to creating CEP extensions with Webpack and React.
Other
87 stars 15 forks source link

Restores file loader from Create React App #2

Closed kennethormandy closed 7 years ago

kennethormandy commented 7 years ago

This restores the file-loader from the current master of Create React App, which restores support for loading in additional files into the build directory. It also removes the need for the SVG-specific loader.

Note that this also means the path is static/media/[name].[hash:8].[ext] where before it would have just been media/[name].[hash:8].[ext], so if you were to run the example app the file would now be in static/media/logo.whatever123.svg instead of media/logo.whatever123.svg, I’m not sure if that was a change in Create React App or if you intentionally had a different directory for a reason I’m not familiar with. I left the url-loader config in place since I wasn’t entirely sure about that, but it would probably make sense to keep them the same, so I can make that change based on your advice.

fusepilot commented 7 years ago

I think my reasoning for getting rid of the static folder was that it would keep the build directory only one level deep. And I also wanted to keep the extendscript folder at the same level as the js folder since they seem equally important.

├── CSXS
│   └── manifest.xml
├── css
│   ├── main.css
│   └── main.css.map
├── extendscript
│   └── index.jsx
├── js
│   ├── main.js
│   └── main.js.map
├── media
│   └── logo.svg
└── panel.html

Instead of

├── CSXS
│   └── manifest.xml
├── extendscript
│   └── index.jsx
├── panel.html
└── static
    ├── css
    │   ├── main.9a0fe4f1.css
    │   └── main.9a0fe4f1.css.map
    ├── js
    │   ├── main.275a4b13.js
    │   └── main.275a4b13.js.map
    └── media
        └── logo.5d5d9eef.svg

Your idea of keeping this part in sync with CRA is a good idea though. What are your thoughts after taking this into consideration?

kennethormandy commented 7 years ago

Hey, thanks for the quick review. Even though just /media makes sense to me, I think anything that can remain the same as in CRA will probably make the project easier to maintain. Also, you should still be able to use whatever folder structure you want in development, so this shouldn’t even be a breaking change.

Mainly I wanted to make sure that there wasn’t something relating to CEP extensions that only allowed directories one level deep.

If I update the PR so both the file-loader and the url-loader directory match CRA, would that work for you?

fusepilot commented 7 years ago

I think anything that can remain the same as in CRA will probably make the project easier to maintain.

Agreed. Lets keep it in sync with CRA then.

Mainly I wanted to make sure that there wasn’t something relating to CEP extensions that only allowed directories one level deep.

Nope. I just like flat folder structures.

If I update the PR so both the file-loader and the url-loader directory match CRA, would that work for you?

That would be great.

Thanks!

kennethormandy commented 7 years ago

8eeef21 changes the build directory and filename to match CRA, I left the other config of the URL loader the same.

kennethormandy commented 7 years ago

Actually I’m not sure if it makes sense to make this change anymore. To actually keep it in sync with CRA, everything should be in static/ including the built CSS and JS.

I changed publicPath to ../ which resolved the pathing issue I was having with url()s in the CSS file, so I will revisit this PR once I understand that config a bit better.