Skip to content

feat(ChunkExtractor): support publicPath override#292

Merged
gregberge merged 1 commit into
gregberge:masterfrom
brkalow:feat/chunk-extractor-public-path
Apr 5, 2019
Merged

feat(ChunkExtractor): support publicPath override#292
gregberge merged 1 commit into
gregberge:masterfrom
brkalow:feat/chunk-extractor-public-path

Conversation

@brkalow

@brkalow brkalow commented Apr 1, 2019

Copy link
Copy Markdown
Contributor

Summary

Add support for passing publicPath to ChunkExtractor. This will allow consumers to override publicPath at runtime.

Currently in our usage we are loading the stats file, manually overriding the publicPath, and then injecting that stats object into ChunkExtractor. This seems like a simple enough change that it warrants being supported out-of-the-box.

Formatted docs section: https://deploy-preview-292--loadable.netlify.com/docs/server-side-rendering/#override-statspublicpath-at-runtime

closes #269

Test plan

Added new test cases to packages/server/src/ChunkExtractor.test.js.

@ryanboswell

Copy link
Copy Markdown

This looks great, exactly what we were hoping for 👍

@gregberge

Copy link
Copy Markdown
Owner

@brkalow thank you for this PR! Could you please rebase and I will merge it.

@brkalow brkalow force-pushed the feat/chunk-extractor-public-path branch from 91e0136 to 259bd6e Compare April 3, 2019 19:45
@brkalow

brkalow commented Apr 3, 2019

Copy link
Copy Markdown
Contributor Author

@neoziro Done, thanks!

@lauterry

lauterry commented Apr 5, 2019

Copy link
Copy Markdown

Hi

I think this is what I was looking for.

My use case is that I call chunkExtractor.getCssString on a server at runtime to gather the critical CSS and inject it in the html before sending it to the browser.

However, now, the output is incorrect therefore when getCssString tries to look for a css file, it didn't find it. Indeed, The outputPath on build time is not the same on runtime.

@neoziro Can you confirm that chunkExtractor.getCssString relies on stats.outputPath to find the css ?

If so, please publish a new release for the new feature

Thank a lot

@brkalow

brkalow commented Apr 5, 2019

Copy link
Copy Markdown
Contributor Author

@lauterry I believe ChunkExtractor already supports passing in outputPath to solve the issue you are having. It looks like ChunkExtractor.getCssString does rely on stats.outputPath to get the filepath:

See here: https://github.com/smooth-code/loadable-components/blob/master/packages/server/src/ChunkExtractor.js#L186

@lauterry

lauterry commented Apr 5, 2019

Copy link
Copy Markdown

Thx @brkalow

You're right. I'll give it a try.

Sorry for the noise. I have confounded publicPath and outputPath

@gregberge gregberge merged commit 9731e9c into gregberge:master Apr 5, 2019
@brkalow brkalow deleted the feat/chunk-extractor-public-path branch April 5, 2019 19:41
@lauterry

lauterry commented Apr 5, 2019

Copy link
Copy Markdown

@brkalow

Indeed, your proposition fixed my issue. Thx

@sourabhkheterpal

Copy link
Copy Markdown

@brkalow Hello, When you guys plan to release this to NPM ? It still gives us older version. Kindly update NPM if possible. Thanks.

@brkalow

brkalow commented Apr 17, 2019

Copy link
Copy Markdown
Contributor Author

@sourabhkheterpal I believe this is included in @loadable/server@5.8.0 👍

@gregberge

Copy link
Copy Markdown
Owner

Yes it is!

@srigar

srigar commented Mar 21, 2020

Copy link
Copy Markdown

@brkalow This is awesome feature what i'm looking for currently. Added public path in chunk extractor but still js(chunk files) not loading from the runtime path which provided in publicPath.
Did I missed anything here?

extractor = new ChunkExtractor({
    statsFile: path.resolve(process.cwd(), './build/loadable-stats.json'),
    publicPath: process.env.CHUNK_ASSET_PATH
});

<ChunkExtractorManager extractor={extractor}>
        {component}
 </ChunkExtractorManager>

@natterstefan

Copy link
Copy Markdown

Hi @srigar,

I have the same issue, did you find a solution yet?

Thanks for your help.

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.

Support "on the fly" public path definitions from webpack

7 participants