Skip to content

Prevent invalid js generation for module names with an apostrophe: - #3602

Closed
matthew-hilty wants to merge 1 commit into
purescript:masterfrom
matthew-hilty:fix-invalid-js-syntax-for-purs-module-names-with-apostrophe
Closed

matthew-hilty wants to merge 1 commit into
purescript:masterfrom
matthew-hilty:fix-invalid-js-syntax-for-purs-module-names-with-apostrophe

Conversation

@matthew-hilty

Copy link
Copy Markdown
Contributor

Formerly, the purescript code

module Foo' where
foo = "foo"

module Bar where
import Foo'

would be parsed without error but would generate the following invalid javascript:

var Foo' = require("../Foo'/index.js");

The present commit prevents this error in two ways:

  1. requiring that module and qualifier names comprise alphanumeric characters only;
    (I've seen some past discord among ps developers about this restriction,
    but it seems the safest choice and code currently in Parser/Lexer.hs (specifically, 'validUName')
    suggests that's the present naming policy.)
  2. applying 'identCharToText' to ModuleNames
    to ensure that the rendered javascript variable names contain no invalid chars,
    should module-naming conventions ever change.

Formerly, the purescript code

    module Foo' where
    foo = "foo"

    module Bar where
    import Foo'

would be parsed without error but would generate the following invalid javascript:

    var Foo' = require("../Foo'/index.js");

The present commit prevents this error in two ways:
    1. requiring that module and qualifier names comprise alphanumeric characters only;
    2. applying 'identCharToText' to ModuleNames
       to ensure that the rendered javascript variable names contain no invalid chars,
       should module-naming conventions ever change.
@hdgarrood

Copy link
Copy Markdown
Contributor

The approach seems okay, but I’m tempted to say that we should wait until the cst work is in before addressing this.

@natefaubion

Copy link
Copy Markdown
Contributor

purescript-cst does handle this check.

@matthew-hilty
matthew-hilty deleted the fix-invalid-js-syntax-for-purs-module-names-with-apostrophe branch April 13, 2019 22:10
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