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.

Sign up for GitHub

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

Jump to bottom

CJS export is missing __esModule - Code transpiled to CJS can't use default export #12

Closed
agilgur5 opened this issue Jul 16, 2019 · 3 comments · Fixed by #24
Closed

CJS export is missing __esModule - Code transpiled to CJS can't use default export #12

agilgur5 opened this issue Jul 16, 2019 · 3 comments · Fixed by #24
Labels
kind: bug Something isn't working properly or is fixed progress: external blocker This is blocked by an issue in an external dependency solution: workaround available There is a workaround available for this

Comments

@agilgur5
Copy link
Owner

agilgur5 commented Jul 16, 2019 •
edited
Loading

See #3 (comment) onward.

This is a bug in tsdx (jaredpalmer/tsdx#165) caused by caused by this config option in its rollup config

This bug makes the v0.1.0 release unintentionally breaking (to an extent at least, as CJS support wasn't available before v0.1.0) and as such fixing it should be high prio. The workaround is to not use default exports, but default is used in the README and v0.1.0 should maintain backward compatibility (though minor releases prior to v1 are allowed to be breaking, this was unintentionally breaking)
@agilgur5 agilgur5 added bug progress: external blocker This is blocked by an issue in an external dependency solution: workaround available There is a workaround available for this labels Jul 16, 2019
@agilgur5
Copy link
Owner Author

agilgur5 commented Jul 17, 2019 •
edited
Loading

Oh wait... default export actually isn't listed in the README. I have used it in code blocks when responding to people though and believe I went back-and-forth on whether to use default in the README or not. Guess it's a good thing I didn't given there are more issues with it than I knew of.

Still breaks backward compatibility and is an unexpected bug in tsdx, but not that high prio given it's not in the README and has a workaround available.

@agilgur5
Copy link
Owner Author

Sent a PR out to tsdx to fix this: jaredpalmer/tsdx#327 .

Tried a simpler/quicker fix back in July, but that failed upon testing. Decided to try a different fix after some renewed frustrations as a result of that bug and that PR does indeed work now!

@agilgur5
Copy link
Owner Author

Would like to release the fix to this in v0.1.x so that it's fixed in the same minor it was broken and is fixed in all future minors.

That would mean before #16 lands as it'll be a minor release. If jaredpalmer/tsdx#327 isn't merged and released soon, I've created a branch on my fork at agilgur5/tsdx#dist that has the change and is installable via npm i -D tsdx@github:agilgur5/tsdx#dist. It can be used in the meantime then.

@agilgur5 agilgur5 mentioned this issue Dec 4, 2019
Merged
agilgur5 added a commit that referenced this issue Dec 4, 2019
@agilgur5
(env/deps): use agilgur5/tsdx fork for esModuleInterop etc
ed27858 
- this uses my fork which merged my PR/commit that fixes the missing
  __esModule in CJS build output
  - which has been an issue in this library since adopting tsdx, as
    that's still an undocumented bug in tsdx
    - the bug means that default imports get transpiled to x = require
      instead of { default: x } = require
      - which affects anyone using CJS and importing the default export
        - and most apps still transpile down, as do most test runners,
          so basically it affects almost everyone using the default
    - for more details, see #12
  - until this fix is merged and released in the root library, will
    be using this fork

- it also has my recently merged PR/commit that changes the cacheRoot
  to ./node_modules/.cache/ instead of ./rts2_cache_* directories
  - can remove them from gitignore and they no longer clutter app root
agilgur5 added a commit that referenced this issue Dec 4, 2019
@agilgur5
(env/deps): use agilgur5/tsdx fork for esModuleInterop etc
3ab52c4 
- this uses my fork which merged my PR/commit that fixes the missing
  __esModule in CJS build output
  - which has been an issue in this library since adopting tsdx, as
    that's still an undocumented bug in tsdx
    - the bug means that default imports get transpiled to x = require
      instead of { default: x } = require
      - which affects anyone using CJS and importing the default export
        - and most apps still transpile down, as do most test runners,
          so basically it affects almost everyone using the default
    - for more details, see #12
  - until this fix is merged and released in the root library, will
    be using this fork

- it also has my recently merged PR/commit that changes the cacheRoot
  to ./node_modules/.cache/ instead of ./rts2_cache_* directories
  - can remove them from gitignore and they no longer clutter app root
@agilgur5 agilgur5 added the kind: bug Something isn't working properly or is fixed label Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind: bug Something isn't working properly or is fixed progress: external blocker This is blocked by an issue in an external dependency solution: workaround available There is a workaround available for this
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

v0.1.3 -- (env/deps): use agilgur5/tsdx fork for esModuleInterop etc agilgur5/mst-persist
1 participant
@agilgur5