vitejs / vite

Next generation frontend tooling. It's fast!
http://vite.dev
MIT License
68.76k stars 6.21k forks source link

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

Open appsforartists opened 1 year ago

appsforartists commented 1 year ago

Description

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

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

stackblitz[bot] commented 1 year ago

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

sapphi-red commented 1 year ago

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) https://github.com/vitejs/vite/blob/66caef369dbe80bb3fbaabdae5ffb05c42124f32/packages/vite/src/node/plugins/importAnalysis.ts#L654-L657

appsforartists commented 1 year ago

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.

https://github.com/vitejs/vite/blob/66caef369dbe80bb3fbaabdae5ffb05c42124f32/packages/vite/src/node/server/transformRequest.ts#L255-L259

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 commented 1 year ago

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

appsforartists commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

sapphi-red commented 1 year ago

Reopening as the PR was reverted #14866 due to breakage: https://github.com/vitejs/vite/pull/14851#issuecomment-1791941779

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

appsforartists commented 1 year ago

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 commented 12 months ago

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 commented 12 months ago

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 commented 11 months ago

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 commented 10 months ago

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.

// 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 commented 10 months ago

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

connorgmeehan commented 10 months ago

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 commented 10 months ago

@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 commented 10 months ago

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

connorgmeehan commented 10 months ago

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 commented 10 months ago

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 commented 9 months ago

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 commented 6 months ago

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 commented 3 months ago

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

tonitrnel commented 2 months ago

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