search

villander / ember-engines-router-service

Provides the Router service for ember-engines
MIT License
14 stars 6 forks source link

fix artefact generation bumping @embroider/* packages #73

Closed SergeAstapov closed 2 years ago

SergeAstapov commented 2 years ago

closes #67.

the root cause of the problem is that initializer file dist/_app_/initializers/ember-engines-router-service.js published to NPM has following contents:

export { default } from "ember-engines-router-service/initializers/ember-engines-router-service.js";

and it should be

export { default } from "ember-engines-router-service/initializers/ember-engines-router-service";

As dist folder gets generated at build time and handled by rollup (technically by rollup plugins), bumping @embroider/addon-dev package to the latest fixes this issue.

@villander could you please release 4.0.1 once this and #72 gets merged? (just a reminder Release Process described in RELEASE.md)

villander commented 2 years ago

@SergeAstapov is there some way to write a test to it to make sure this issue won't happen again, please?

SergeAstapov commented 2 years ago

@villander tbh I don't experience that issue with v0.4.0 and per https://github.com/villander/ember-engines-router-service/issues/67#issuecomment-1253285115 root cause of an issue in something else.

@villander we also already have tests in this repo and all of them are good..

@villander can we anyways land this and https://github.com/villander/ember-engines-router-service/pull/72 and release a new version?

villander commented 2 years ago

Given the #67 issue hasn't been solved, I don't think is worth we merge it. I know we have good test coverage for the service itself given I wrote all of them. But given this issue is regarding the build pipeline, we don't have any node-tests that make sure it's working as expected.

villander commented 2 years ago

@SergeAstapov any update here?

SergeAstapov commented 2 years ago

@villander I successfully updated ember-engines-router-service to 0.4.0 and see no issues in our app. Not sure what you meant by node tests, feel free to add some if you have an idea how those may look like.

tbh I see no issue merging and publishing this anyways alongside https://github.com/villander/ember-engines-router-service/pull/72

villander commented 2 years ago

@SergeAstapov are you saying that #67 is not an issue? If it is not an issue why do we need this PR here? I'm just trying to understand which page we are on here.

SergeAstapov commented 2 years ago

digged further in issue reported in #67 and appears this is not the fix of the root cause.