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

mapCommentType is block for CSS files #20

Closed
wants to merge 1 commit into from

Conversation

jamesarosen
Copy link

@jamesarosen
Copy link
Author

Some caveats:

  1. mapCommentType is still dictated purely by this library and cannot be changed by a client. This can easily be fixed by changing the line to opts.mapCommentType || ..., but I wanted to make the smallest change that could possibly work.
  2. mapCommentType is dependent upon CSS files being named *.css
  3. mapCommentType is CSS-aware, but mapFile is not
  4. I haven't added a test, though I'm working on it.

@stefanpenner
Copy link
Collaborator

mapCommentType is dependent upon CSS files being named *.css

this seems reasonable

@stefanpenner
Copy link
Collaborator

@rwjblue @ef4 r?

@ef4
Copy link
Owner

ef4 commented Jan 15, 2016

The upstream caller (ember-cli) always knows what kind of files it's concatenating. I would rather have it pass an appropriate option than try to autodetect based on extensions.

@jamesarosen
Copy link
Author

The immediate upstream caller is ef4/broccoli-sourcemap-concat. Is that where you'd like a PR?

@ef4
Copy link
Owner

ef4 commented Jan 15, 2016

Yes, please. Or perhaps https://github.com/ember-cli/broccoli-concat instead. @stefanpenner, is that practical to use for everybody at this point?

@jamesarosen
Copy link
Author

jamesarosen commented Jan 15, 2016 via email

@rwjblue
Copy link
Collaborator

rwjblue commented Jan 15, 2016

Broccoli-Concat also says it doesn't yet support sourcemaps.

From the title of the README:

Broccoli concatenator that generates & propagates sourcemaps

This filter is designed to be fast & good enough. It can generates source maps substantially faster than you'll get via mozilla/source-map, because it's special-cased for straight line-to-line contenation.

@jamesarosen
Copy link
Author

I could've sworn there was an open issue/PR that was basically "Broccoli doesn't do sourcemaps right yet, so ¯\_(ツ)_/¯." But I can't find it, so 🎉

@stefanpenner
Copy link
Collaborator

We should switch to broccoli-concat for 1.13. Although we will hopefully cutover to 2x soon

@jamesarosen
Copy link
Author

@stefanpenner do you want me to make a go at that? Any advice?

@stefanpenner
Copy link
Collaborator

@jamesarosen if you want, whats preventing you from using the 2.x betas?

@jamesarosen
Copy link
Author

@stefanpenner we're on Ember 1.13. I don't know if those are compatible.

@stefanpenner
Copy link
Collaborator

@stefanpenner we're on Ember 1.13. I don't know if those are compatible.
Show all checks

Yes they are compatible, the only major difference is 2.x bundles different blueprints

I would recommend upgrading, I do not believe we will back port this change.

stefanpenner pushed a commit to stefanpenner/fast-sourcemap-concat that referenced this pull request Apr 25, 2016
@dfreeman
Copy link

Did an issue/PR for this ever make it to broccoli-concat? I don't see one, but wanted to make sure there aren't plans to handle this in a different layer that have come together since this thread last had activity.

@jamesarosen
Copy link
Author

I just opened broccolijs/broccoli-concat#58

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

5 participants