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

Fix issue incorrect paths for fonts. Add methods to Use relative paths. #95

Closed
wants to merge 100 commits into from
Closed

Conversation

nikrou
Copy link

@nikrou nikrou commented Jul 14, 2017

The idea is to allow to use relative path for fonts and/or images.

nikrou and others added 30 commits July 14, 2017 17:25
This PR was squashed before being merged into the master branch (closes #102).

Discussion
----------

Fix missing tsloader return statement

This commit (35d9e6f#diff-168726dbe96b3ce427e7fedce31bb0bc) removed inadvertently the `return this;` statement from the `enableTypeScriptLoader` function.

This prevents calling another function chained on `enableTypeScriptLoader` in a `webpack.config.js` file.

This PR just puts the `return this;` statement back.

Commits
-------

51cf2ab Fix missing tsloader return statement
when using dev-server and an absolute URL for publicPath
to get webpack encore working in docker environment
test for successful config set when using devServer and setPublicPath
with an absolute URL
- when using devServer and absolute URL for publicPath
- also add test case
…e autoprexer (weaverryan)

This PR was merged into the master branch.

Discussion
----------

Fixing a bug where @import CSS files were not put through the autoprexer

Fixes #98

By default, when you use `@import`, loaders are not applied. This fixes that: using the postcss-loader when it's used. The functional test proves the fix.

Commits
-------

35c7e6d Fixing a bug where @import CSS files were not put through the autoprefixer
This PR was squashed before being merged into the master branch (closes #99).

Discussion
----------

Refactor plugin configuration

Extract encore built in plugin configuration logic into plugin utils.

First step toward allowing to be able to configure plugins through options and have more modular integration for plugins inside encore. From now on only public API with proper options is needed,
for most cases.

This changes are internals and tries to improve plugin architecture inside encore. Please take a look on it and feedback are welcome :). So far I see improvements for several issues:

* #2 - Point .2 ...for plugins we should do the same as for loaders, from point of view of module split (utilities).
* #63 - Better (more modular) integration of the plugin as well its configuration.
* #65 - Same as #63
* #79 - Now it should be possible to pass options and/or check for flags to apply plugin or not.
* #87 - Some small steps to make clean plugin more configurable. Public API still need to be changed though.

thanks in advance.

Commits
-------

65adfdc Refactor plugin configuration
…ausk, weaverryan)

This PR was merged into the master branch.

Discussion
----------

Relaxed public path requirements with dev-server

Fixes #59 and finishes #66.

* Adds a new `--keep-public-path` option for `dev-server`. When used, your `publicPath` is not prefixed with the dev server URL. For #59, this means you can use `setPublicPath('/build')` with the dev-server, and your assets will remain local (i.e. `/build/main.js` instead of `http://localhost:8080/build/main.js`).

* It is now possible to pass an absolute URL to `setPublicPath()` without an error when using `dev-server`. But, we issue a big warning, because this means your assets will point to to that absolute URL, instead of to the dev-server (which for most setups, is not what you want).

@samjarrett I'd love to confirm that this would solve your issue in Docker :).

Commits
-------

4bc1e19 Using real public path, though it doesn't look like it matters
92e22af Allowing an absolute publicPath with dev-server, but showing a warning
830fdb5 Adding --keep-public-path to dev-server to allow you to fully control the publicPath
b27f7c9 Reversing some of the changes we won't do for now, and adding the failing test
e206a12 fix issue in generated public path of manifest.json
eb5565b convert error into warning
910b6bc convert error into warning
This PR was merged into the master branch.

Discussion
----------

Add an EditorConfig file

This PR adds an [.editorconfig](http://editorconfig.org/) file at the root of the project with the following settings:

* Unix line endings
* For JS files:
  * 4-spaces indents
  * Trim trailing whitespaces
  * Insert a new line at the end of each file

Commits
-------

a26fa15 Add EditorConfig file
…weaverryan)

This PR was squashed before being merged into the master branch (closes #114).

Discussion
----------

Fixing a bug with webpack 3.4.0 with extra arg as an entry

Fixes #112

The updated way avoids triggering the "singleton" behavior or yargs. This is important so that later, when webpack uses yargs, it uses the updated `process.argv` (because we remove our extra "command" argument so that webpack doesn't see it).

I'm not sure exactly which dep/commit caused the issue, but this a more proper solution anyways.

Also added a smoke test for the encore binary.

Commits
-------

2b7ca5b Fixing a bug with webpack 3.4.0 with extra arg as an entry
This isn't a perfect escape, but it's just for test paths
…fault assets loaders (Lyrkan)

This PR was squashed before being merged into the master branch (closes #110).

Discussion
----------

Always add a hash to the name of the files created by the default assets loaders

This PR modifies the configuration of the default assets loaders so a `[hash]` is always added to the output file names, even if versioning is disabled.

It basically prevents having two files with the same name (but in a different directory) overwriting each other during build.

This closes #73 and was further discussed in #103.

Commits
-------

d5cd482 Use [hash:8] for images and fonts filenames instead of [hash]
874235d Modify images/fonts loaders so a hash is always added to the name of output files
Lyrkan and others added 20 commits October 10, 2017 18:47
This PR was squashed before being merged into the master branch (closes #152).

Discussion
----------

Add various methods to configure default plugins

This PR adds some methods to configure default plugins (closes #148, closes #87 and closes #15):

* `Encore.configureDefinePlugin(callback)`
* `Encore.configureExtractTextPlugin(callback)`
* `Encore.configureFriendlyErrorsPlugin(callback)`
* `Encore.configureLoaderOptionsPlugin(callback)`
* `Encore.configureUglifyJsPlugin(callback)`

Other changes:

* Allows the `clean-webpack-plugin` to be configured using `Encore.cleanupOutputBeforeBuild(paths, callback)`
* Fixed a small mistake in the `Encore.configureManifestPlugin()` jsdoc
* Reorganized flags/callbacks in the constructor of `webpack.config.js` since it was starting to be a bit hard to read. I'm not sure about the way I splitted things though, let me know what you think about it.
* Renamed `common-chunks.js` to `commons-chunks.js` since the name of the plugin is `CommonsChunkPlugin`

@weaverryan: Not directly related but while doing all of that I noticed that `sassOptions` uses snake-case whereas I used camel-case in #144 for `enablePreactPreset()` and in #115 for `configureRuntimeEnvironment()`, should we do something about that?

***Edit 1:** Added `Encore.configureCleanWebpackPlugin()`*
***Edit 2:** Removed `Encore.configureCleanWebpackPlugin()` to use `Encore.cleanupOutputBeforeBuild(paths, callback)` arguments instead*

Commits
-------

286787a Minor text changes
f72614b Remove configureCleanWebpackPlugin and use cleanupOutputBeforeBuild arguments instead
90c8565 Add various methods to configure default plugins
…ecognized API property (Lyrkan)

This PR was squashed before being merged into the master branch (closes #150).

Discussion
----------

Add a 'Did you mean ...' error message in case of an unrecognized API property

This PR aims to improve the error message displayed when an user makes a typo in the name of one of the API methods he's trying to use.

Since pictures speak louder than words:

**Before**
![2017-08-30_19-01-27](https://user-images.githubusercontent.com/850046/29885253-724d45a0-8db6-11e7-8ec5-9e63fa9d786e.png)

**After**
![2017-08-30_19-03-44](https://user-images.githubusercontent.com/850046/29885266-7a0f3d0c-8db6-11e7-9af8-d9aad52917c2.png)

Note that the error is triggered when trying to access the property, not when the method is called (since it doesn't exist), hence the "is not a recognized property" message. I'm aware that the use of the "property" word in it may not be ideal, but using "method" instead feels wrong too since we could theoretically have other things than methods in the API object.

Commits
-------

a16e1af Only add the "Did you mean" message if the levenshtein distance is less than 3
1ceabc1 Add "or method" to the "Did you mean ..." error message
d592481 Add a 'Did you mean ...' error message for unrecognized API methods
This PR was squashed before being merged into the master branch (closes #159).

Discussion
----------

Replace "resolve_url_loader" by "resolveUrlLoader"

This PR deprecates the `resolve_url_loader` option used by the `enableSassLoader` and replaces it by `resolveUrlLoader`.

This is done to be consistent with other methods that use camel-case instead of snake-case (see [#152](#152 (comment))).

`resolve_url_loader` will still work for now but will display the following deprecation message:

![2017-09-13_23-38-56](https://user-images.githubusercontent.com/850046/30402523-4c736d86-98de-11e7-97c1-81a216912fb7.png)

Commits
-------

ff442a6 Add a logger.deprecation(message) method
9dac153 Replace sass option 'resolve_url_loader' by 'resolveUrlLoader'
This PR was merged into the master branch.

Discussion
----------

Fix "__esModule is not a recognized property" error

This PR removes the "__esModule is not a recognized property" error that is currently displayed after importing Encore using an ES6 import through a `webpack.config.babel.js` file (fixes #166).

Commits
-------

78ac47a Fix "__esModule is not a recognized property" error when using a webpack.config.babel.js file (#166)
Add an instance of DefinePlugin even in development mode
@stof
Copy link
Member

stof commented Oct 10, 2017

Why is there 100 commits in this PR ? You should not rebase the commits of master on top of yours, as this creates a copy of them (and it does not solve conflicts).

@nikrou
Copy link
Author

nikrou commented Oct 11, 2017

@stof I saw after. I will fix it quickly. Sorry. Do my PR is still revelant or interesting ?

@stof
Copy link
Member

stof commented Oct 11, 2017

@nikrou I don't know, because it is very hard for me to know what it actually does (as the diff shows these 100 commits)

@nikrou
Copy link
Author

nikrou commented Oct 11, 2017

I don't know how to fix that mess. I fix the problem of rebase on my branch :
master...nikrou:issue-88

@Lyrkan
Copy link
Collaborator

Lyrkan commented Oct 11, 2017

Easiest way to do it would probably be to:

  • create a new branch based on the problematic one (e.g. issue-88 => issue-88-copy)
  • remove the original branch locally (issue-88)
  • recreate it based on master
  • cherry-pick your commits from the other branch you created earlier
  • git push -f

But since Github doesn't seem to recognize your repository anymore (did you recreate it?) I'm not sure how to handle that...

By the way I left a comment in the related issue. Is adding setFontsPublicPath() and setImagesPublicPath() the right way to fix it?

@nikrou
Copy link
Author

nikrou commented Oct 11, 2017

Yes I recreaed my repository. It doesn't seem to be a good idea ! :-(

@stof
Copy link
Member

stof commented Oct 11, 2017

yeah, if you deleted you fork, you will have to create a new PR.

@nikrou
Copy link
Author

nikrou commented Oct 11, 2017

My bad. I create another PR : #179

@Lyrkan
Copy link
Collaborator

Lyrkan commented Oct 11, 2017

Ok, let's close this one then.

@Lyrkan Lyrkan closed this Oct 11, 2017
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.

None yet

8 participants