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

Unable to import from a v1 addon in a v2 addon #1179

Closed
runspired opened this issue Apr 6, 2022 · 9 comments
Closed

Unable to import from a v1 addon in a v2 addon #1179

runspired opened this issue Apr 6, 2022 · 9 comments

Comments

@runspired
Copy link

I have a v2 addon that defines several transitions for liquid-fire, which it lists as a peerDependency. Attempting to build results in the following error:

!] Error: 'isAnimating' is not exported by ../../../node_modules/liquid-fire/index.js, imported by src/transitions/move-over-half.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
src/transitions/move-over-half.js (2:26)
1: import $ from "jquery";
2: import { animate, finish, isAnimating, Promise, stop } from "liquid-fire";
                             ^
3: 
4: function biggestSize(context, dimension) {
Error: 'isAnimating' is not exported by ../../../node_modules/liquid-fire/index.js, imported by src/transitions/move-over-half.js

I would have expected that @embroider/addon-dev helped with this situation out of the box. In my plugins I list as expected addon.dependencies() which the code comments led me to believe was for this purpose.

@runspired
Copy link
Author

This is the inverse of #1175 in case that wasn't clear 😅

@runspired
Copy link
Author

runspired commented Apr 6, 2022

I was able to resolve this by using a better rollup config. I'd initially tried mirroring some things that apollo-glimmer had done, but shifted to doing what ember-animated had done since ember-animated also ships colocated components written in typescript. NVM I got trolled by a different build error making it seem like I'd gotten past this, I had not :(

@runspired runspired reopened this Apr 6, 2022
@NullVoxPopuli
Copy link
Collaborator

Can you share your configs, before and after?

@NullVoxPopuli
Copy link
Collaborator

I think maybe related,
I ran in to an issue with a v2 addon importing from an optional peer dependency declared in an app here:
https://github.com/NullVoxPopuli/ember-resources/runs/5842084950?check_suite_focus=true#step:7:428

Since ember-async-data needs to be a v2 addon anyway, that's my path forward for that particular issue.
But, I think it'd be fairly straight-forward to make a scenario test for this 🤔

@runspired
Copy link
Author

@NullVoxPopuli yes that's the same issue, but it'll be a rough thing for the community to disentangle what has and hasn't been migrated yet if we block migration based on whether deps/peerDeps are also all migrated.

I think the issue might be something like that while rollup is being configured to treat the module as external here: https://github.com/embroider-build/embroider/blob/main/packages/addon-dev/src/rollup-addon-dependencies.ts it is still trying to validate that the module actually exists. Either that or the above config doesn't work when using typescript :/

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Apr 6, 2022

rough thing for the community to disentangle what has and hasn't been migrated yet if we block migration based on whether deps/peerDeps are also all migrated.

I agree -- that's why I make test :D

it is still trying to validate that the module actually exists.

well, the issue is that we're running in to an app-buildtime problem and that rollup plugin is an addon build-time thing for the v2 addon.
So I think something may be goofy with how we're preparing things to send off to webpack (hopefully a failing test will tell us more)

@runspired
Copy link
Author

@NullVoxPopuli got this solved locally, you have to pass the addon to rollup as external in the config. It would be nice if the addonDependencies plugin would do this automatically, but it's fairly easy to do manually too. Here's my configs for an addon written in TS that uses components and imports from an external addon in v1.

babel.config.json

{
  "presets": [
    [
      "@babel/preset-typescript"
    ]
  ],
  "plugins": [
    "@embroider/addon-dev/template-colocation-plugin",
    [
      "@babel/plugin-proposal-decorators",
      {
        "version": "legacy"
      }
    ]
  ]
}

rollup.plugin.js

import { Addon } from "@embroider/addon-dev/rollup";
import ts from "rollup-plugin-ts";

const addon = new Addon({
  srcDir: "src",
  destDir: "dist",
});

const globallyAvailable = ["components/**/*.{js,ts}", "helpers/**/*.{js,ts}"];

export default {
  output: addon.output(),

  external: ["liquid-fire"],

  plugins: [
    addon.publicEntrypoints([...globallyAvailable, "transitions/**/*.{js,ts}"]),

    addon.appReexports(globallyAvailable),

    ts({
      transpiler: "babel",
      browserslist: false,
      transpileOnly: true, // private in-mono-repo package, global tsconfig already handles type wiring
      tsconfig: {
        fileName: "../../../tsconfig.json",
        hook: (config) => ({ ...config, declaration: true }),
      },
    }),

    addon.dependencies(),

    addon.hbs(),

    addon.keepAssets(["**/*.css"]),

    addon.clean(),
  ],
};

@runspired
Copy link
Author

Also the dependencies setup here:

  "dependencies": {
    "@embroider/addon-shim": "^1.5.0"
  },
  "peerDependencies": {
    "liquid-fire": "^0.34.0"
  },
  "devDependencies": {
    "@embroider/addon-dev": "^1.5.0",
    "rollup": "^2.70.1",
    "@babel/core": "^7.17.8",
    "@babel/plugin-proposal-class-properties": "^7.16.7",
    "@babel/plugin-proposal-decorators": "^7.17.8",
    "@babel/plugin-transform-typescript": "^7.16.8",
    "@babel/plugin-transform-runtime": "^7.17.0",
    "@babel/preset-typescript": "^7.16.7",
    "@babel/preset-env": "^7.16.11",
    "@babel/runtime": "^7.17.8",
    "@rollup/plugin-babel": "^5.3.1",
    "rollup-plugin-ts": "^2.0.5",
    "typescript": "^4.6.3"
  },
  

@NullVoxPopuli
Copy link
Collaborator

ah interesting -- ok -- I'm going to ensure this works over here then: https://github.com/embroider-build/embroider/pull/1126/files#diff-0d15995080ee9f281ca534fe2ec6e630f4770d419cfc99a68f9f3ebcac778e42

thanks for following up!

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

No branches or pull requests

2 participants