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

Importing public files with /* @vite-ignore */ dynamic imports throws an error #14850

Open
4 tasks done
appsforartists opened this issue Nov 2, 2023 · 23 comments · Fixed by #14851
Open
4 tasks done
Labels
has workaround p2-edge-case Bug, but has workaround or limited in scope (priority)

Comments

@appsforartists
Copy link
Contributor

Description

My project generates JavaScript templates for non-technical users. Before Vite, I published a zip file that looked like this:

  • index.html
  • config.js
  • bundles/
    • mount.js

I'm trying to port this project to Vite.

To prevent Vite from minifying the source and mangling the name, I've tried putting config.js in public. However, when I run vite serve, I'm met with this error message:

Failed to load url /config.js (resolved id: /config.js). This file is in /public and will be copied 
as-is during build without going through the plugin transforms, and therefore should not be 
imported from source code. It can only be referenced via HTML tags.

Reproduction

While I respect the effort to prevent people from making mistakes, there ought to be the ability to say "no really, this is on purpose."

In short, I want to be able to tell Vite "Treat this JS file like any other asset. Don't change its name or its contents, and allow it to be imported from the devserver."

Suggested solution

The easiest solution from both an implementation and a user's POV would be a config flag, like server.allowDangerousPublicDirImports. If it's set, Vite skips throwing the error and serves the requested file as if it's any other asset.

This gives the user the final decision about what is and is not permissible in their architecture, without resorting to crazy backdoors like writing a plugin to serve JS as an assetInclude.

The term Dangerous in the flag name warns people to not set it without consideration. Using a flag gives it an obvious place to live in the documentation to make it easier for people to find. (A nod could also be included in ERR_LOAD_PUBLIC_URL.) I'm not convinced that the risk of serving files from publicDir is so high that they need to individually be whitelisted.

Alternative

Whitelisting, either by:

  1. an inline comment, like import(/* vite-treat-as-asset */ 'config.js), or
  2. a config field, like jsModulesInPublicDir: ['**/*config.js'].

The comment is less discoverable than a config field, but easier to use. You explicitly grant permission at the call site, so there is nothing to maintain. (Imagine if the file name changes.)

A config field is DRYer (for instance, if many packages in a monorepo share a vite.config, one setting repairs the whole project).

Additional context

config.js is effectively a JSON with generous commenting and some dependency injection to provide functionality like shuffling and centering.

It's designed to be hand-edited by people who aren't technical enough to create or maintain a web app, but are capable of editing some strings. All the complexity is in mount.js, which is just a blob to them. They can edit the config file, drag it to a file server, and have an app that meets their needs without touching the command line. Moreover, config can be generated by a web app after mount has already been minted.

Validations

Copy link

stackblitz bot commented Nov 2, 2023

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@appsforartists appsforartists changed the title Allow escape from Vite to import JS Allow opt-in to JS dynamic imports from publicDir Nov 2, 2023
@sapphi-red
Copy link
Member

I think this is a bug.

I guess it'll work if && !hasViteIgnore is added here. (hasViteIgnore would need to be moved outside the if block together)

if (
!urlIsStringRE.test(url) ||
isExplicitImportRequired(url.slice(1, -1))
) {

@sapphi-red sapphi-red added the p3-minor-bug An edge case that only affects very specific usage (priority) label Nov 2, 2023
@appsforartists
Copy link
Contributor Author

Thanks for the speedy reply!

I believe the problem is that the devserver is getting a request for a JS file it doesn't recognize.

? `This file is in ${publicDirName} and will be copied as-is during ` +
`build without going through the plugin transforms, and therefore ` +
`should not be imported from source code. It can only be referenced ` +
`via HTML tags.`
: `Does the file exist?`

I'm new to Vite so I may be mistaken, but it looks like importAnalysis.ts is part of the build infrastructure.

There may also be a problem with importing (leading to hacks like const importWIthoutVite = (p) => import(p)), but this FR is to allow .js requests to pass when the devserver sees they're in publicDir.

@sapphi-red
Copy link
Member

sapphi-red commented Nov 2, 2023

I tested out my change and it worked with your reproduction. So I created the PR: #14851

@appsforartists
Copy link
Contributor Author

Thanks for the knowledge and quick fix!

Should I need to do this?

const importWithoutVite = (path: string) => import(/* @vite-ignore */path);

importWithoutVite('./config.js').then(

If I try to

import(/* @vite-ignore */'./config.js')

directly, I get:

Internal server error: Failed to resolve import "./config.js" from "src/mount.tsx". Does the file exist?

@appsforartists
Copy link
Contributor Author

After further investigation, since build puts the bundle in assets, but serve serves it from /, I end up with a path conflict. Is there a better strategy than handling the manually?

const importWithoutVite = (path: string) => import(/* @vite-ignore */path);

importWithoutVite(
  // handle JS going into `assets` when bundled
  import.meta.env.PROD
    ? '../config.js'
    : './config.js'
).then(

@bluwy
Copy link
Member

bluwy commented Nov 3, 2023

Since you're opting-out of Vite handling normalizing the import paths, you would have to do that manually.

@sapphi-red
Copy link
Member

sapphi-red commented Nov 3, 2023

Reopening as the PR was reverted #14866 due to breakage: #14851 (comment)

The workaround for now would be to inject a middleware that serves the file.
https://discord.com/channels/804011606160703521/804011606160703524/1169905478499844136

@sapphi-red sapphi-red reopened this Nov 3, 2023
@sapphi-red sapphi-red added p2-edge-case Bug, but has workaround or limited in scope (priority) and removed p3-minor-bug An edge case that only affects very specific usage (priority) labels Nov 3, 2023
@sapphi-red sapphi-red changed the title Allow opt-in to JS dynamic imports from publicDir Importing files with /* @vite-ignore */ dynamic imports throws an error Nov 3, 2023
@sapphi-red sapphi-red changed the title Importing files with /* @vite-ignore */ dynamic imports throws an error Importing public files with /* @vite-ignore */ dynamic imports throws an error Nov 3, 2023
@appsforartists
Copy link
Contributor Author

Turns out you can skip the path switching for the devserver, because the devserver mounts at the root, and /../ is the same as /.

So import('../my-asset.js') will work whether you're in /mount.jsx or /assets/mount.js.

@appsforartists
Copy link
Contributor Author

appsforartists commented Nov 29, 2023

Here's a prototype that seems to be working:

// Copyright 2023 Google LLC.
// SPDX-License-Identifier: Apache-2.0


const literalDynamicImports = (literals: Array<string>): PluginOption => {
  let inDevServer = false;

  return {
    name: 'vite-plugin-literal-dynamic-imports',

    configureServer() {
      inDevServer = true;
    },

    resolveDynamicImport(specifier, importer, options) {
      if (literals.includes(specifier as string)) {
        return false;
      }
    },

    transform(code, id, options){
      // Vite only intermediates when the dev server is on, so we only have to
      // work around it when it's on.
      if (inDevServer) {
        // `MagicString` tracks string modifications to generate a matching source
        // map.
        const magicCode = new MagicString(code);

        for (let result of code.matchAll(/import\(([^\)]*?('([^']+?)'|"([^"]+?)")[^\)]*?)\)/g)) {
          const specifier = result[3] ?? result[4];
          const startIndex = result.index!;
          const endIndex = startIndex + result[0].length;

          magicCode.update(
            startIndex,
            endIndex,
            // `.split('').join('')` is a no-op, but it prevents
            // `importAnalysis` from analyzing (and flagging) the specifier.
            //
            // `@vite-ignore` disables the warning that dynamically generated
            // strings won't be analyzed.
            `import(/* @vite-ignore */ "${ specifier }".split('').join(''))`,
          );
        }

        return {
          code: magicCode.toString(),
          map: magicCode.generateMap({}),
        };
      }
    },
  };
};

RegExes suck - I don't promise it's perfect, but it's working for me.

Does this look good? If so, I can publish it.

@appsforartists
Copy link
Contributor Author

Dammit - just tried upgrading to Vite@5.0.4 and this error is back:

Failed to load url /experiment-config.js (resolved id: /experiment-config.js). This file is in 
../passthrough and will be copied as-is during build without going through the plugin transforms, 
and therefore should not be imported from source code. It can only be referenced via HTML tags.

@appsforartists
Copy link
Contributor Author

I feel like I keep going down this route, I'm gonna spend more time that I can afford reverse engineering Vite's dev server to figure out precisely what I need to do in configureServer to make this work. @bluwy, do you have tips for me?

It seems that Vite has no problem serving the file (so I don't need to call sirv), but that the caller is the one throwing the error. That part is confusing to me. I don't know why I can load the imported JS file directly without causing the error, but opening the HTML+JS bundle that imports it does throw the error.

transformMiddleware appears to be what's calling the erroring loadAndTransform, but it's doing it with a path to the imported file. If that was the source of the error, I'd expect requesting the imported file directly in the browser would cause the error. The error only occurs when imported from HTML+JS, but I don't know how/why.

@ghost
Copy link

ghost commented Jan 14, 2024

Here's how I solved it for my little use case in a vite react app...

I added 2 files to a subfolder in public.

  • public/app/
    • config.js
    • importer.js
// config.js
export const myPlugin = {
  id: "yolo",
  main(id) {
    console.log("Yolo!", id);
  },
};
// importer.js
function importNative(path) {
  return import(path);
}

Then in my index.html file I added <script src="/app/importer.js"></script> to the <head> area.

To test it in my src/main.tsx file I did:

const importNative = (path: string) => {
  // path = import.meta.env.BASE_URL + path; // IF you have a base path
  const inative = window["importNative"] as (path: string) => any;
  return inative(path);
};

importNative("/app/config.js").then(config => {
  const myPlugin = config?.myPlugin;
  if (!myPlugin) { console.log("Config?", config); return; }
  myPlugin.main(myPlugin.id);
});

@bartoszrudzinski
Copy link

Any progress with that? After updating to Vite 5 dynamic import throws an error and @vite-ignore is not working properly.

@connorgmeehan
Copy link

connorgmeehan commented Jan 24, 2024

I was able to work around this issue by creating a custom import function for when I want to import a js file from the public folder. Works for me with vite: ^5.0.8.

/**
 * This is a patched version of the native `import` method that works with vite bundling.
 * Allows you to import a javascript file from the public directory.
 */
function importStatic(modulePath) {
    if (import.meta.env.DEV) {
        return import(/* @vite-ignore */ `${modulePath}?${Date.now()}`);
    } else {
        return import(/* @vite-ignore */ modulePath);
    }
}

const path = '/content/foo.js`; //  -> `/public/content/foo.js`
const module = await importStatic(path);

Edit: @appsforartists raised a point that the ${Date.now()} suffix is not required and a simple ? suffix will work, but the full ?${Date.now()} may help avoid any caching issues when running in dev mode.

Edit 2: Also worth mentioning, if you want to do relative imports for static files you'll have to make sure vite is bundling JS files into the root of the dist/ folder and not the dist/assets/ folder like it was for me. You can do this by adjusting the rollup config

export default defineConfig({
    build: {
        rollupOptions: {
            output: {
                chunkFileNames: '[name]-[hash].js',
                entryFileNames: '[name]-[hash].js',
                assetFileNames: '[name]-[hash][extname]',
            }
        }
    }
})

@appsforartists
Copy link
Contributor Author

@waynebloss - clever! You could inline importer into the HTML to skip a download.

@connorgmeehan Did you try that in both vite serve and vite build? I feel like that's where I started, but IIRC, I was seeing error overlays in vite serve.

Does the ?url get Vite to skip the path validation? You might be better with Date.now, so you're always getting a fresh copy of modulePath

@appsforartists
Copy link
Contributor Author

I just tested @connorgmeehan's workaround in the reproduction, and adding a trailing ? does seem to work around this.

@connorgmeehan
Copy link

Yep it worked for me for both serve as well as build/preview (again, only tested with ^5.0.8) Is the the Date.now required to avoid caching of the module (I'm guessing in instances where you're hot-reloading)?

import(/* @vite-ignore */ `${modulePath}?{Date.now()}`)

@appsforartists
Copy link
Contributor Author

Yeah. I wouldn't say it's "required," but it can be nice to ensure you're getting unique, uncached URLs on each load.

I haven't played with it in this context to see how well it plays with HMR. It's just an old habit to avoid caching.

@appsforartists
Copy link
Contributor Author

The workaround seems really fragile. I tried to implement it in the literalDynamicImports function above and was getting errors. I tried other techniques to make it dynamic like .split('').join('') and got errors.

We seem to be lucky that this one sneaks by Vite now, but I don't trust that to stay in the future. Regardless, I'm glad to be on a stable version now (rather than monkeypatching my local module with sapphi-red's PR).

Thanks again Connor!


One of the errors was Unknown variable dynamic import. I forgot to write the other one down.

@mrchantey
Copy link

Not sure if this is exactly the same problem as above but in case its useful here's the workaround I'm using, in a web component that allows users to specify wasm files:

    const base = this.src.startsWith('http') ? undefined : window.location.href
    const src = new URL(this.src, base)
    const module = await import(/* @vite-ignore */src.href)

@unit-404
Copy link

Issue still exists, I unable to enforce optional dynamic import.

@tonitrnel
Copy link

For me, await import(/* @vite-ignore */`${location.origin}/xxx.js`) is work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has workaround p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants