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 broken image import in bundled build. #3174

Closed
wants to merge 23 commits into from

Conversation

cain-wang
Copy link

@cain-wang cain-wang commented Apr 16, 2021

The line I removed set resolveProxyImports to false for bundled builds and cause image and json files to be imported directly instead of through proxy imports.

// build/dist/App.js
import logo from "./logo.svg";

This change fixes:

[BUG] sowpack bundle build generate import statement for images and json files #3109
#3109

Changes

This change prevents optimize.bundle to automatically set buildOptions.resolveProxyImports to false.

Testing

Tested bundled builds with @snowpack/app-template-react and the the proxy imports is properly resolved to:

// build/dist/logo.svg.proxy.js
var logo_svg_proxy_default = "/dist/logo.svg";

Docs

Bug fix only

The line I removed set resolveProxyImports to false for bundled builds and cause image and json files to be imported directly instead of through proxy imports.

This change fixes:

[BUG] sowpack bundle build generate import statement for images and json files FredKSchott#3109
FredKSchott#3109
@cain-wang cain-wang requested a review from a team as a code owner April 16, 2021 19:36
@vercel
Copy link

vercel bot commented Apr 16, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/3DzhKP8jE9wAs8bbQp9pjP8NBA4i
✅ Preview: https://snowpack-git-fork-cain-wang-patch-1-pikapkg.vercel.app

re-run create-snowpack-app.test.js
drwpow and others added 15 commits April 16, 2021 16:53
Esbuild requires publicPath to be a remote URL. Only pass if that’s true.
* first fixtures tests

* Adds dedent for string literal tests

* Migrates named import test

* Migrates cdn url test

* Migrates config alias test

* Remove unused mock-app index.html file

* Create teemplate test

* Migrates config custom path test

* Migrates config env test

* Migrates extends plugins test [WIP]

* Migrates config packageOptions.external test

* Moves plugin objectExtends into plugins sub folder

* Migrates config plugins instantiatedObject test

* Migrates config loading withExtension tests [WIP]

* Migrates metaUrlPath with mounted directory tests

* Migrates mount tests

* Run format on codebase

* Migrates config optimize test

* Migrates config out test

* Migrates config package lookup fields test

* Remove snowpack as dependency from alias test

* Fixup suite and test names

* Migrates optimize treeshake test

* Migrates css modules test

* Migrates another metaUrlPath test

* Migrates config invalid test

* Migrates html env variables test [WIP]

* Migrates import dot folder test

* Migrates glob test

* Migrates import json test

* Migrates import ts test

* Migrates jsx inject test

* Migrates module resolution test

* Fix: Make loadConfiguration check for files in provided root

* Make test runner use loadConfiguration

* Skip invalid config values test

* Unskip non-esm config loading tests

* Add package json with deps to ts import test

* Migrates package bootstrap test

* Adds snowpack cache to test result output

* Migrates source remote-next test

* Migrates package tippy test

* Migrates package workspace test [WIP]

* Migrates plugin build script test

* Migrates plugin svelte test

* Migrates plugin hook transform test

* Migrates plugin run script test [WIP]

* Migrates plugin sass test

* Migrates plugin vue test

* Migrates module resolve js test

* Migrates scan imports html test

* Migrates utf8 test

* Set logger level to error to clean up test output

* Remove old loading from cjs tests

* Migrates config loading from JSON test

* Remove old loading from package json tests

* Use toContain consistently

* Try using readFileSync to prevent CI failures

* Break up glob test and don't use snapshots

* Fix path of CSS file in moduleResolution test

* Try use array-flatten over array-is in moduleResolution test

* Add content to template file to try fix readFile error

* Try using babel core over CLI for buildScript test

* Favour snowpack config over overrides in tests

* Use babel core and CLI in plugin buildScript test

* Fixup template test with snowpack config

* Rebase and fix new skipped tests

* Skip failing bootstrap test and checkout old test

* Skip failing build script test and checkout old test

* Skip failing config error test and checkout old test

* Document reason for skipped tests

* Replace back with forward slashes on windows

* Actually convert windows paths to posix paths

* Skip absolute path test

* Replace process exit with throw and unskip tests

* Add env as optional key in SnowpackUserConfig

* Fix config env variables not being injected into HTML

* Fix plugin extends test by passing dir to dotenv

* Pass posix path as TEMP_TEST_DIRECTORY

* Try using TEMP_TEST_DIRECTORY for absoulte out path

* Mostly empty commit to rerun tests

* Delete now migrated and passing old CLI tests

* Removes old CSS modules test

Co-authored-by: Fred K. Schott <fkschott@gmail.com>
* improve node_modules excluding

* test: update config-mount test

* feat: improve node_modules exclusions

* refactor: cleaner support for mounting node_modules

* test: update config/mount tests

Co-authored-by: natemoo-re <nate@natemoo.re>
Subsequent loadUrls() for files in same directory would not await completion of loading of directory, leading to 404 Not Found errors.
* Recommend http2-proxy for API proxying

* Update WebSocket example
* Take the result of the first plugin

This fixes `runPipelineLoadStep` so that it takes the first plugin result, rather than running them all.

Fixes FredKSchott#3183

* Return result instead

* Update the snapshot
The line I removed set resolveProxyImports to false for bundled builds and cause image and json files to be imported directly instead of through proxy imports.

This change fixes:

[BUG] sowpack bundle build generate import statement for images and json files FredKSchott#3109
FredKSchott#3109
re-run create-snowpack-app.test.js
@cain-wang
Copy link
Author

Sorry, my git is a bit messed up mixing both personal and work accounts. Will try to create a new pull request.

@cain-wang cain-wang closed this Apr 21, 2021
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.

8 participants