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

External source map not getting generated with correct file name #455

Closed
balajisivanath opened this issue Jul 18, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@balajisivanath
Copy link

commented Jul 18, 2018

Given the following rollup configuration, external css source map is getting generated with file name as "bundle2.css" instead of "bundle.css.map"

import css from "modular-css-rollup";
export default {
    input: "source/exports.js",
    output: {
        file: "out/bundle.js",
        format: "iife",
        name: "test",
        sourcemap: true,
        assetFileNames: "bundle.css" //If this is not given, then modular-css-rollup outputs the file assets/bundle-[hash].css
    },
    plugins: [
        css({
            map: {
                inline: false
            }
        })
    ]
};

annotated

Rollup version: 0.62.0
Modular css version: 14.0.1

@tivac

This comment has been minimized.

Copy link
Owner

commented Jul 18, 2018

What happens if you use assetFileNames : "[name][extname]" instead?

It's very similar to what I do in the tests

const assetFileNames = "assets/[name][extname]";

and that looks to generate the expected name based on

it("should generate external source maps", async () => {
const bundle = await rollup({
input : require.resolve("./specimens/simple.js"),
plugins : [
plugin({
namer,
map : {
inline : false,
},
}),
],
});
await bundle.write({
format,
assetFileNames,
file : prefix(`./output/rollup/external-source-maps/simple.js`),
});
// Have to parse it into JSON so the propertyMatcher can exclude the file property
// since it is a hash value and changes constantly
expect(JSON.parse(read("./rollup/external-source-maps/assets/simple.css.map"))).toMatchSnapshot({
file : expect.any(String),
});
});

@balajisivanath

This comment has been minimized.

Copy link
Author

commented Jul 18, 2018

What happens if you use assetFileNames : "[name][extname]" instead?

Changing to this format generates file correctly 👍

@tivac

This comment has been minimized.

Copy link
Owner

commented Jul 18, 2018

Weirdly I don't see anywhere in my code that is generating the 2.css version containing the map code, though I can replicate what you're seeing.

I'd argue that the original config value being a hardcoded filename is an incorrect value to pass, but ideally it wouldn't explode things... 😥

@balajisivanath

This comment has been minimized.

Copy link
Author

commented Jul 18, 2018

Ouch! Sourcemapping url in bundle.css isn't replaced with correct filename
image

Weirdly I don't see anywhere in my code that is generating the 2.css version containing the map code, though I can replicate what you're seeing.
I'd argue that the original config value being a hardcoded filename is an incorrect value to pass, but ideally it wouldn't explode things... 😥

I guess issue could be in rollup, considering that assetFileName config is under their experimental feature list

@tivac

This comment has been minimized.

Copy link
Owner

commented Jul 18, 2018

Well that sourceMappingURL is definitely a problem. Ugh, the interactions between PostCSS and rollup are a real thing sometimes.

I can fix that one, at least!

tivac added a commit that referenced this issue Jul 18, 2018

fix: replace placeholders in assetFileNames
So that external source map references point to the right file. Gotta pass a real file to postcss or it does some funny things.

Fixes #455

tivac added a commit that referenced this issue Jul 18, 2018

fix: replace placeholders in assetFileNames
So that external source map references point to the right file. Gotta pass a real file to postcss or it does some funny things.

Fixes #455

tivac added a commit that referenced this issue Jul 19, 2018

fix: replace placeholders in assetFileNames
So that external source map references point to the right file. Gotta pass a real file to postcss or it does some funny things.

Fixes #455

@tivac tivac closed this in #457 Jul 19, 2018

tivac added a commit that referenced this issue Jul 19, 2018

fix: External map references (#457)
* fix: replace placeholders in assetFileNames

So that external source map references point to the right file. Gotta pass a real file to postcss or it does some funny things.

Fixes #455
@tivac

This comment has been minimized.

Copy link
Owner

commented Jul 24, 2018

@balajisivanath

This comment has been minimized.

Copy link
Author

commented Jul 24, 2018

Latest release v14.1.0 is built with this changeset 119e10c. Above fix is merged after that, hence it is not there.

@tivac

This comment has been minimized.

Copy link
Owner

commented Jul 24, 2018

Hrmm, not sure how that missed the 14.1.0 release since I intended it to be part of it. My mistake, I'll try to get to that soonish.

@tivac

This comment has been minimized.

Copy link
Owner

commented Jul 25, 2018

Released in modular-css-rollup@14.2.0, sorry about the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.