addon-blueprint icon indicating copy to clipboard operation
addon-blueprint copied to clipboard

gts files are left with side-effecting imports

Open NullVoxPopuli opened this issue 1 year ago • 11 comments

I currently get this messaging in my rollup build when updating to the latest blueprint:

[js] (!) Unused external imports
[js] TOC imported from external module "@ember/component/template-only" but never used in "src/components/dialog.gts", "src/components/-private/typed-elements.gts", "src/components/external-link.gts", "src/components/form.gts", "src/components/portal-targets.gts", "src/components/progress.gts", "src/components/shadowed.gts", "src/components/portal.gts", "src/components/popover.gts", "src/components/toggle.gts" and "src/components/switch.gts".
[js] ModifierLike,WithBoundArgs imported from external module "@glint/template" but never used in "src/components/dialog.gts", "src/components/progress.gts", "src/components/popover.gts" and "src/components/switch.gts".
[js] default imported from external module "@ember/routing/router-service" but never used in "src/components/link.gts".
[js] Middleware,MiddlewareData imported from external module "@floating-ui/dom" but never used in "src/components/popover.gts".
[js] Signature imported from external module "ember-velcro/modifiers/velcro" but never used in "src/components/popover.gts".
[js] (!) Generated an empty chunk
[js] "template-registry"
[js] created dist in 738ms

Repro here: https://github.com/universal-ember/ember-primitives/pull/114

Example output:

       │ File: dist/components/switch.js
───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ import { fn, hash } from '@ember/helper';
   2   │ import { on } from '@ember/modifier';
   3   │ import { cell } from 'ember-resources';
   4   │ import { uniqueId } from '../utils.js';
   5   │ import { Label } from './-private/typed-elements.js';
   6   │ import { toggleWithFallback } from './-private/utils.js';
   7   │ import { templateOnly } from '@ember/component/template-only';
   8   │ import '@glint/template';
   9   │ import { precompileTemplate } from '@ember/template-compilation';
  10   │ import { setComponentTemplate } from '@ember/component';
  11   │ 

The main issue being the remaining import @glint/template -- which is a type-only package, and in the real code, I have:

import type { WithBoundArgs } from '@glint/template';

So the whole thing should be removed in the emitted js

NullVoxPopuli avatar Sep 07 '23 03:09 NullVoxPopuli

I have a reduced example here: https://stackblitz.com/edit/stackblitz-starters-rbahsf?description=Starter%20project%20for%20Node.js,%20a%20JavaScript%20runtime%20built%20on%20Chrome%27s%20V8%20JavaScript%20engine&file=babel.config.json,package.json,tsconfig.json,rollup.config.mjs,src%2Findex.ts,dist%2Findex.js&title=node.new%20Starter

Having a hard time reproducing the example -- it appears to have something to do with my code, as my simple examples don't reproduce the issue, but when I copy my addon's src folder into the default v2 addon blueprint output, the issue reproduces.

NullVoxPopuli avatar Sep 07 '23 04:09 NullVoxPopuli

I have a repro using the v2 addon blueprint here: https://stackblitz.com/edit/stackblitz-starters-qner5x?

output dist/index.js

import '@glint/template';
import Component from '@glimmer/component';

class Hello extends Component {}

export { Hello };
//# sourceMappingURL=index.js.map

NullVoxPopuli avatar Sep 26 '23 14:09 NullVoxPopuli

It happens when you forget import type!!

So,

import type { Thing } from 'whatever';

will be fully removed

but

import { type Thing } from 'whatever';

will not be.

so we probably need a lint to prefer the import type

NullVoxPopuli avatar Sep 26 '23 14:09 NullVoxPopuli

Please, re-open this issue as it is not fixed.

Repro repo here with stackblitz app https://github.com/bartocc/stackblitz-starters-pyxvkz

When stackblitz has finished launching the app, check the file my-addon/dist/index.js, it'll contain the culprit import '@glint/template';

This only happens with the .gts extension. Change my-addon/src/index.gts to my-addon/src/index.ts and the bug is gone

bartocc avatar Sep 27 '23 10:09 bartocc

I'm having the same issue here: https://github.com/lukasnys/ember-radix-ui.

No matter if I use import { WithBoundArgs } from '@glint/template';, import type { WithBoundArgs } from '@glint/template'; or import { type WithBoundArgs } from '@glint/template';. The import '@glint/template'; in the dist remains causing the consuming test-app to crash.

lukasnys avatar Sep 30 '23 08:09 lukasnys

Made a stackblitz of the above repo if folks don't want to clone: https://stackblitz.com/edit/github-yl7xy1?file=packages%2Faccordion%2Fsrc%2Fcomponents%2Faccordion.gts&file=packages%2Faccordion%2Fdist%2Fcomponents%2Faccordion.js image

NullVoxPopuli avatar Oct 05 '23 17:10 NullVoxPopuli

resolved by ensuring that content-tag in your lockfile is up to date (at least 1.1.2). This'll be fixed shortly with a new minimum @embroider/addon-dev version.

NullVoxPopuli avatar Oct 07 '23 17:10 NullVoxPopuli

I confirm that using content-tag 1.1.2 fixes the issue 👍 thx @NullVoxPopuli and @ef4 for the fix 👍

bartocc avatar Oct 09 '23 08:10 bartocc

Same for me, I tried updating to [email protected] in the Stackblitz example @NullVoxPopuli provided and then it's fixed 👍. Thanks!

lukasnys avatar Oct 09 '23 17:10 lukasnys

Same here: updating content-tag removed the import '@glint/template' line, thanks :clap:!


But I just found that we have a similar issue with ember-concurrency, where a import 'ember-concurrency'; remains in the compiled output... is it maybe related or should it be addressed in EC?

Some details
  • Using latest EC @ 3.1.1
  • Added EC babel transform to babel.config.json "ember-concurrency/lib/babel-plugin-transform-ember-concurrency-async-tasks" (https://github.com/machty/ember-concurrency/blob/master/lib/babel-plugin-transform-ember-concurrency-async-tasks.js)

Build output

[js] 
[js] > [email protected] build:js
[js] > rollup --config
[js] 
[types] 
[types] > [email protected] build:types
[types] > glint --declaration
[types] 
[js] 
[js]  → dist...
[js] (!) Unresolved dependencies
[js] https://rollupjs.org/troubleshooting/#warning-treating-module-as-external-dependency
[js] ember-concurrency/-private/async-arrow-runtime (imported by "src/components/hello.gts")
[js] ember-concurrency (imported by "src/components/hello.gts")
[js] (!) Generated empty chunks
[js] "index" and "template-registry"
[js] (!) Unused external imports
[js] task imported from external module "ember-concurrency" but never used in "src/components/hello.gts".
[js] created dist in 1.1s
[js] npm run build:js exited with code 0
[types] npm run build:types exited with code 0

Component / GTS

import { task } from 'ember-concurrency';

export default class Hello extends Component<Signature> {
  // ...
  testTask = task({ drop: true }, async () => {
    console.log('hello');
  });
}

Compiled / JS

import { buildTask } from 'ember-concurrency/-private/async-arrow-runtime';
import 'ember-concurrency';

....

class Hello extends Component {
  constructor(...args) {
    super(...args);
    _defineProperty(this, "testTask", buildTask(() => ({
      context: this,
      generator: function* () {
        console.log('hello');
      }
    }), {
      drop: true
    }, "testTask", null));
  }

enspandi avatar Oct 09 '23 18:10 enspandi

yeah, sounds like a bug in their babel plugin -- they have enough knowledge of what they need to do to remove that themselves (babel can't do it, because it's not safe), but ember-concurrency knows that there is no reason to use a side-effecting import from itself.

NullVoxPopuli avatar Oct 09 '23 18:10 NullVoxPopuli