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

Support comments in dynamic imports #535

Closed
FredKSchott opened this issue Jun 23, 2020 · 8 comments
Closed

Support comments in dynamic imports #535

FredKSchott opened this issue Jun 23, 2020 · 8 comments
Labels
contributors welcome! contributors welcome! good first issue Good for newcomers

Comments

@FredKSchott
Copy link
Owner

FredKSchott commented Jun 23, 2020

See: FredKSchott/create-snowpack-app#152

Webpack supports configuring code-split bundles via comment:

      const mod1 = await import(
        /* webpackChunkName: "lang-de" */ "./languagePacks/de"
      );

While we don't care too much about matching webpack support, it would be nice to preserve these comments for anyone connecting a webpack bundler to their final build. Right now, Snowpack's import resolver appears to strip comments and convert this sort of import directly to:

const mod1 = await import("./languagePacks/de");
@FredKSchott FredKSchott added contributors welcome! contributors welcome! good first issue Good for newcomers labels Jun 23, 2020
@ematipico
Copy link
Contributor

I'd like to help on this one

@FredKSchott
Copy link
Owner Author

That would be great! I think the first step would be finding out where this is happening: I thought that it was us doing this but now I'm not so sure. We actually don't seem to touch dynamic imports unless they are ONLY a string via scanCodeImportsExports(), so an import like this would pass right by our import scanner/resolver.

Does Babel do this by default? Or esbuild?

@izengliang
Copy link

ionic react use dynamic imports .

import ReactDOM from "react-dom";
import React, { FC } from "react";
import {IonContent} from "@ionic/react";

const App: FC = () => {
  return <div><IonContent>Hello world</IonContent></div>;
};

ReactDOM.render(<App />, document.body);

  [error] [404] /web_modules/common/%22%20+%20bundleId%20+%20%22.entry.js%22%20+%20'.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.js" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.jsx" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.ts" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.tsx" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.js" + '.jsx

@sohonisaurabh
Copy link
Contributor

I was looking into this. No great progress, however, when I found the following thing with babel presets:

  1. @babel/preset-react - Retains the comment
  2. @babel/preset-typescript - Retains the comment
  3. @babel/preset-env - Deletes the comment

Here is the small example of repl:
https://babeljs.io/en/repl#?browsers=&build=&builtIns=false&spec=false&loose=false&code_lz=IYZwngdgxgBAZgV2gFwJYHsI1QWwA7oBOyAwpiMsBMiABQCUMA3gFAwxTnIddU0wBeGMADuwVN1wFitGAHoAVDBEBTAEZ5gUANYkAFkm0A5YDhUAuGACIANlQDmAWgAmKqzAVyA5ADo5nCAo-EC96AG42HkD0GxUfG3R7WgCg6hAfAAkASXCWAF8WFikiUl40hgigA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=true&sourceType=module&lineWrap=true&presets=env%2Creact&prettier=true&targets=&version=7.10.3&externalPlugins=

@FredKSchott If this clicks something in your head, I can create a PR. Its just that I am new to the codebase so not sure where we are wiring different babel presets.

@sohonisaurabh
Copy link
Contributor

Also, there was a problem with rollup which they fixed quite early. Reference here: rollup/rollup#2778

@FredKSchott
Copy link
Owner Author

Curious of @babel/preset-env is doing this if you're not touching dynamic imports. The repl seems to be rewriting import() to require(), which could be why you're seeing that

@FredKSchott
Copy link
Owner Author

Okay, dug into this a bit more and it's definitely coming from esbuild, and based on this convo I don't think the project is interested in adding comment preservation: evanw/esbuild#221

Some options to solve:

  • If you're just using JS, we no longer pass your code through esbuild (it was essentially a no-op), so this should be resolved for you starting in v2.7.0 releasing later this week
  • If you're using TS, JSX, etc. you'll need to use Babel as your build step which should preserve comments for you. See: https://www.snowpack.dev/#babel

@331000738
Copy link

331000738 commented Nov 19, 2020

ionic react use dynamic imports .

import ReactDOM from "react-dom";
import React, { FC } from "react";
import {IonContent} from "@ionic/react";

const App: FC = () => {
  return <div><IonContent>Hello world</IonContent></div>;
};

ReactDOM.render(<App />, document.body);

  [error] [404] /web_modules/common/%22%20+%20bundleId%20+%20%22.entry.js%22%20+%20'.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.js" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.jsx" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.ts" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.tsx" + '.js
    ✘ /Users/liangzeng/Documents/web/node_modules/.cache/snowpack/dev/common/" + bundleId + ".entry.js" + '.jsx

add this code to your "index.html", then the problem will be resoved:

<script type="module" src="https://cdn.jsdelivr.net/npm/@ionic/core/dist/ionic/ionic.esm.js"></script>
<script nomodule src="https://cdn.jsdelivr.net/npm/@ionic/core/dist/ionic/ionic.js"></script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributors welcome! contributors welcome! good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants