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

rebase assets url #5

Closed
wants to merge 2 commits into from
Closed

Conversation

iAdramelk
Copy link
Member

Code for #3

@iAdramelk iAdramelk changed the title Url rebase rebase assets url Aug 17, 2014
@MoOx
Copy link
Contributor

MoOx commented Aug 20, 2014

I think instead of using a plugin in a plugin we should stick with a standalone plugin for that (if we can do it that way).
It can make sense if the plugin that rewrite url can just use from & to option of the postcss instance.

postcss-import correctly keep sourcemap so we already have from informations for every imported parts.

I think a standalone plugin can be a good idea to do the following:

  • rebase url() using source objects (& from properties). it to is available somewhere (we can make a postcss update for that if not available) we can adapt all path from each from to global to value.
  • inline url() if an option is passed.

This way we can have a simple plugin we can call... postcss-url ? (since it will handle url()) ? What do you think of that ?

postcss-rewriteurls & postcss-helpers seems like overkill (too much code to me for what we really need & can do using simple stuff like native node path.resolve().

@iAdramelk
Copy link
Member Author

Hmm. But what if the user didn't want/need source maps and don't set option for them then creating postcss instance? (if we don't have from param we can't deduce it ourself.)

Also if he enables source map creation for using this plugin, but didn't want to deploy actual source map to the server, then he will have number of problems:

  1. If I understand correctly, then you set { map: true } and save css file, it will write path to the map to the end of the created css file. If there is no such file on the server, it will throw error to console.
  2. Parsing and manipulations with source maps is more slow, then without it (possible reason to disable them on big projects). So if we say that source maps are necessary to rewrite paths, then we make this slowing necessary too.

So I think it probably better to do inside this plugin. We can inline all the necessary code instead of using external plugin, but it will harder to maintain. See my comment there: #6 (comment)

Lets think how to do it better.

@MoOx
Copy link
Contributor

MoOx commented Aug 21, 2014

You miss a thing.
We do not need sourcemap file or similar. When postcss parse files it keep reference of the source in each rules as source that contains a file property (used here for example

options.from = styles.rules[0].source.file
)
Like said here postcss/postcss#81 (comment) I don't think we need lots of helpers to handle a simple resolve of path.
I will do that in this plugin but using a simpler thing that will be more straightforward.

@MoOx MoOx closed this in dd3c4e7 Aug 24, 2014
@MoOx
Copy link
Contributor

MoOx commented Aug 24, 2014

I'm made a plugin that don't require a lot of code to handle that

https://github.com/postcss/postcss-url/blob/5290b328006cff3548e3592e28cd2bbecab7dfe9/index.js#L42-L66

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.

2 participants