Skip to content

Include full file path in CJS requires.#3314

Merged
LiamGoodacre merged 1 commit intopurescript:masterfrom
chexxor:full-path-requires
Apr 23, 2018
Merged

Include full file path in CJS requires.#3314
LiamGoodacre merged 1 commit intopurescript:masterfrom
chexxor:full-path-requires

Conversation

@chexxor
Copy link
Copy Markdown
Contributor

@chexxor chexxor commented Apr 21, 2018

Fixes #2621

Changes the JS output from require("../SomeModule") to require("../SomeModule/index.js"), and same for the implicit FFI imports.

stack test is proving difficult. First time I ran it, it had trouble installing bower dependencies, giving ECONFLICT Unable to find suitable version for purescript-either. After manually installing them, I got errors about Monoid being a duplicate module, which occurred because it's been moved to Prelude in the latest version (so we may need to adjust the version range in the bower file). After manually deleting Monoid stuff from the PS Prelude it was using, I get test failures caused by stuff missing from Prim -- Module Prim.RowList was not found., and same for Prim.Ordering and Prim.Symbol.

Copy link
Copy Markdown
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@chexxor
Copy link
Copy Markdown
Contributor Author

chexxor commented Apr 21, 2018

Ah, I bet some of these problems are fixed in #3312, as the deps I've been tinkering with appear to have been bumped and fixed in that PR. Also, it looks like stuff missing from Prim could be fixed by that PR, as it's moving them from lib to compiler (if I read it right).

The other question I have is about tests -- do we need to have some tests written to assert this? If you ask me, I don't think it's necessary.

@garyb
Copy link
Copy Markdown
Member

garyb commented Apr 21, 2018

Yeah, everything will be failing until we finish with #3312, that's the problem with #3309 too.

The tests will be subject to the changes you've made here already, so no, there's no further specific tests I can think of that will be necessary :)

@chexxor
Copy link
Copy Markdown
Contributor Author

chexxor commented Apr 21, 2018

Here's why I want this patch, BTW. https://www.screencast.com/t/KgVyC3ksG or https://1drv.ms/v/s!AnIo6jtaL-CjkG-zDxXZxVHIc-8s

No need to add a bundle or transpile step PureScript code during development -- just refer directly to the single PS module and the dependencies will be dynamically loaded. (Yeah, SystemJS does transpile, but it seems to be much easier to get started with, I think.)

The bundling step is often a hurdle for Haskell people coming to PureScript. Yes, tooling like pulp --to x.js can help with that, but it complicates the project set-up. Also this patch to the compiler doesn't break anything and is a benefit-only change -- it adds the ability to very easily use SystemJS in-browser loader instead of a server-side bundler.

@LiamGoodacre LiamGoodacre merged commit 8ae2a06 into purescript:master Apr 23, 2018
@LiamGoodacre
Copy link
Copy Markdown
Member

Thanks!

@chexxor chexxor deleted the full-path-requires branch April 23, 2018 14:56
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

Successfully merging this pull request may close these issues.

3 participants