-
Notifications
You must be signed in to change notification settings - Fork 107
Type imports #208
base: master
Are you sure you want to change the base?
Type imports #208
Changes from 3 commits
6297f6d
05378b3
aff5cb3
8366c0c
6ea93da
9eff946
d4d4452
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import {TypeTwo} from './typeTwo' | ||
|
||
|
||
type SampleOne extends TypeTwo { | ||
validate() { true } | ||
} | ||
type TypeOne extends String { | ||
validate() { true } | ||
} | ||
type Alpha extends String { | ||
validate() { this.test(/^[a-zA-Z]*$/) } | ||
} | ||
|
||
type Alphanumeric { | ||
validate() { this.test(/^[a-zA-Z0-9]*$/) } | ||
alphaValue: String | ||
} | ||
|
||
type Ascii extends String { | ||
validate() { this.test(/^[\x00-\x7F]+$/) } | ||
} | ||
|
||
type Secondary extends String { | ||
validate() { true } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
|
||
type TypeTwo extends String { | ||
validate() { this.test(/^[a-z]*$/) } | ||
} | ||
|
||
type Fourth extends String { | ||
validate() { this.test(/^4$/)} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// Lvl 1 - simply import everything. | ||
import * from './subModule/typeOne' | ||
// Lvl 2 - import single item only | ||
import {Alpha, Alphanumeric} from './subModule/typeOne' | ||
// Lvl 3 - import as an aliased type | ||
import {Alpha, Alphanumeric} as types from './subModule/typeOne' | ||
// Lvl 4 - import recursive types | ||
import { SampleOne } from './subModule/typeOne' | ||
|
||
// Lvl 5 - import with an adjusted path | ||
// import from './subModule/typeOne' at '/somewhere' // duplicate import test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: This will be done in a seperate module. |
||
|
||
|
||
type Test extends Another { | ||
validate() { this.test(/^[0-9]*$/) } | ||
} | ||
|
||
type Another { | ||
validate() { this.message.length < 14 } | ||
something : String | ||
} | ||
path / is SampleOne { | ||
validate() { true } | ||
} | ||
|
||
path /test is Alphanumeric { | ||
read() {true} | ||
} | ||
|
||
path /alpha is Alpha { | ||
read() {true} | ||
} | ||
|
||
path /alphanumeric is types.Alphanumeric { | ||
read() { true } | ||
} | ||
|
||
path /ascii is Ascii { | ||
read() { true } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
let rulesParser = require('./rules-parser'); | ||
let fs = require('fs'); | ||
/* | ||
Imports file parser for split file systems | ||
Note: Using a modified ES6 syntax to include imports | ||
*/ | ||
export function parseWithImports(filename: string) { | ||
// creating a stream through which each file will pass | ||
let contents = fs.readFileSync(filename, "utf8"); | ||
return parserWrapper(contents, filename); | ||
} | ||
|
||
/* | ||
*************************** Function Section **************************** | ||
*/ | ||
|
||
/* ******** wrapper for recursive parsing ******* */ | ||
function parserWrapper(data: string, filename: string) { | ||
var sym = rulesParser.parse(data); | ||
|
||
// Process any imports in symbol list | ||
for (let i = 0; i < sym.imports.length; i++) { | ||
let next = sym.imports[i]; | ||
var nextFilename = getNextFilenameFromContextAndImport(filename, next.filename); | ||
let contents = fs.readFileSync(nextFilename, 'utf8'); | ||
let nextSymbols = parserWrapper(contents, nextFilename); | ||
sym.imports[i].symbols = nextSymbols; // add it to the next part of the tree | ||
} | ||
return sym; | ||
}; // end function | ||
|
||
// Convert absolute filenames to relative | ||
// Convert relative filenames to include original path | ||
function getNextFilenameFromContextAndImport(current: string, nextImport: any) { | ||
current = current.replace('.bolt', ''); | ||
nextImport = nextImport.replace('.bolt', ''); | ||
var currentFn = current.split('/'); | ||
var nextFn = nextImport.split('/'); | ||
let result = ''; | ||
if (nextFn[0] !== '.' && nextFn[0] !== '..') { // global reference | ||
result = './node_modules/' + nextImport + '/index.bolt'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make the assumption that this is always ran from the root of the project directory? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, interesting point. The difference here is between bower & npm (or npm dedupe). My preference would be not to have nesting (bower) but I didn't think of the nesting module issue. I can go back to the drawing board and look at the module resolution process to see if we can support an npm hierarchy module resultion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to support nesting, but maybe I'm not familiar enough is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. node_modules are a standard npm spec so if they use NPM this would exist in the project root directory (not ./src/node_modules). It's a valid test to load it from the ./src/myfile.bolt directory but generally the root directory is still considered ./ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will have a look at using the following package to do the resolution https://www.npmjs.com/package/node-modules-resolve There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or this one is probably better https://www.npmjs.com/package/resolve There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. It looks like that first package just recursively searches parent directories until it gets to a If the root directory is considered to be the current working directory we should check for it when we start. This makes an assumption which isn't enforced anywhere as far as I can tell. |
||
} else { | ||
// import {./something} -> ['.','something'] -> '' | ||
// import {./something/anotherthing} -> ['.','something','anotherthing'] -> something | ||
currentFn.pop(); // remove trailing file name and leave only the directory | ||
nextFn = currentFn.concat(nextFn); | ||
// if file.bolt exists then we have it otherwise return | ||
if (fs.existsSync(nextFn.join('/') + '.bolt')) { | ||
result = nextFn.join('/') + '.bolt'; | ||
} else { | ||
result = nextFn.join('/') + '/index.bolt'; | ||
} | ||
} | ||
return result; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think that if you read from
STDIN
then you would just resolve relative to the current directory instead of the file location. Thoughts? I'm not sure what the best thing for global imports would be... Maybe they should look in global node packages only?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I didn't really think of std in as a use case. Not sure if this is required though. See comments below about global modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least we should gracefully handle errors if people try and do this, but having these two code paths isn't great either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think throwing a deprecation warning is appropriate but I wouldn't remove it unless there is a major version bump (backward compatability must be maintained). So I would add this as an open issue to remove old behaviour in the next major version bump.
So 2 suggestions, I will add the deprecation warning and can add the deprecation issue for the next major milestone. (you might have to create the next major milestone tag).